Many DRM drivers have common code to make a stub connector implementation that wraps a drm_panel. By wrapping the panel in a DRM bridge, all of the connector code (including calls during encoder enable/disable) goes away.
v2: Fix build with CONFIG_DRM=m, drop "dev" argument that should just be the panel's dev, move kerneldoc up a level and document _remove().
Signed-off-by: Eric Anholt eric@anholt.net Acked-by: Daniel Vetter daniel.vetter@ffwll.ch --- Documentation/gpu/drm-kms-helpers.rst | 6 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/bridge/Kconfig | 11 +- drivers/gpu/drm/bridge/lvds-encoder.c | 157 ++++----------------------- drivers/gpu/drm/bridge/panel.c | 197 ++++++++++++++++++++++++++++++++++ include/drm/drm_bridge.h | 7 ++ 6 files changed, 238 insertions(+), 141 deletions(-) create mode 100644 drivers/gpu/drm/bridge/panel.c
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index c075aadd7078..7c5e2549a58a 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -143,6 +143,12 @@ Bridge Helper Reference .. kernel-doc:: drivers/gpu/drm/drm_bridge.c :export:
+Panel-Bridge Helper Reference +----------------------------- + +.. kernel-doc:: drivers/gpu/drm/bridge/panel.c + :export: + .. _drm_panel_helper:
Panel Helper Reference diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index c156fecfb362..4cc9c02cc3f2 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -24,6 +24,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm-$(CONFIG_PCI) += ati_pcigart.o drm-$(CONFIG_DRM_PANEL) += drm_panel.o +drm-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm-$(CONFIG_OF) += drm_of.o drm-$(CONFIG_AGP) += drm_agpsupport.o drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index f6968d3b4b41..c4daca38743c 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -4,6 +4,14 @@ config DRM_BRIDGE help Bridge registration and lookup framework.
+config DRM_PANEL_BRIDGE + def_bool y + depends on DRM_BRIDGE + select DRM_KMS_HELPER + select DRM_PANEL + help + DRM bridge wrapper of DRM panels + menu "Display Interface Bridges" depends on DRM && DRM_BRIDGE
@@ -27,8 +35,7 @@ config DRM_DUMB_VGA_DAC config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF - select DRM_KMS_HELPER - select DRM_PANEL + select DRM_PANEL_BRIDGE help Support for transparent parallel to LVDS encoders that don't require any configuration. diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c index f1f67a279426..0903ba574f61 100644 --- a/drivers/gpu/drm/bridge/lvds-encoder.c +++ b/drivers/gpu/drm/bridge/lvds-encoder.c @@ -8,144 +8,18 @@ */
#include <drm/drmP.h> -#include <drm/drm_atomic_helper.h> -#include <drm/drm_connector.h> -#include <drm/drm_crtc_helper.h> -#include <drm/drm_encoder.h> -#include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_bridge.h> #include <drm/drm_panel.h>
#include <linux/of_graph.h>
-struct lvds_encoder { - struct device *dev; - - struct drm_bridge bridge; - struct drm_connector connector; - struct drm_panel *panel; -}; - -static inline struct lvds_encoder * -drm_bridge_to_lvds_encoder(struct drm_bridge *bridge) -{ - return container_of(bridge, struct lvds_encoder, bridge); -} - -static inline struct lvds_encoder * -drm_connector_to_lvds_encoder(struct drm_connector *connector) -{ - return container_of(connector, struct lvds_encoder, connector); -} - -static int lvds_connector_get_modes(struct drm_connector *connector) -{ - struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector); - - return drm_panel_get_modes(lvds->panel); -} - -static const struct drm_connector_helper_funcs lvds_connector_helper_funcs = { - .get_modes = lvds_connector_get_modes, -}; - -static const struct drm_connector_funcs lvds_connector_funcs = { - .dpms = drm_atomic_helper_connector_dpms, - .reset = drm_atomic_helper_connector_reset, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = drm_connector_cleanup, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -static int lvds_encoder_attach(struct drm_bridge *bridge) -{ - struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge); - struct drm_connector *connector = &lvds->connector; - int ret; - - if (!bridge->encoder) { - DRM_ERROR("Missing encoder\n"); - return -ENODEV; - } - - drm_connector_helper_add(connector, &lvds_connector_helper_funcs); - - ret = drm_connector_init(bridge->dev, connector, &lvds_connector_funcs, - DRM_MODE_CONNECTOR_LVDS); - if (ret) { - DRM_ERROR("Failed to initialize connector\n"); - return ret; - } - - drm_mode_connector_attach_encoder(&lvds->connector, bridge->encoder); - - ret = drm_panel_attach(lvds->panel, &lvds->connector); - if (ret < 0) - return ret; - - return 0; -} - -static void lvds_encoder_detach(struct drm_bridge *bridge) -{ - struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge); - - drm_panel_detach(lvds->panel); -} - -static void lvds_encoder_pre_enable(struct drm_bridge *bridge) -{ - struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge); - - drm_panel_prepare(lvds->panel); -} - -static void lvds_encoder_enable(struct drm_bridge *bridge) -{ - struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge); - - drm_panel_enable(lvds->panel); -} - -static void lvds_encoder_disable(struct drm_bridge *bridge) -{ - struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge); - - drm_panel_disable(lvds->panel); -} - -static void lvds_encoder_post_disable(struct drm_bridge *bridge) -{ - struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge); - - drm_panel_unprepare(lvds->panel); -} - -static const struct drm_bridge_funcs lvds_encoder_bridge_funcs = { - .attach = lvds_encoder_attach, - .detach = lvds_encoder_detach, - .pre_enable = lvds_encoder_pre_enable, - .enable = lvds_encoder_enable, - .disable = lvds_encoder_disable, - .post_disable = lvds_encoder_post_disable, -}; - static int lvds_encoder_probe(struct platform_device *pdev) { - struct lvds_encoder *lvds; struct device_node *port; struct device_node *endpoint; - struct device_node *panel; - - lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL); - if (!lvds) - return -ENOMEM; - - lvds->dev = &pdev->dev; - platform_set_drvdata(pdev, lvds); - - lvds->bridge.funcs = &lvds_encoder_bridge_funcs; - lvds->bridge.of_node = pdev->dev.of_node; + struct device_node *panel_node; + struct drm_panel *panel; + struct drm_bridge *bridge;
/* Locate the panel DT node. */ port = of_graph_get_port_by_id(pdev->dev.of_node, 1); @@ -161,29 +35,34 @@ static int lvds_encoder_probe(struct platform_device *pdev) return -ENXIO; }
- panel = of_graph_get_remote_port_parent(endpoint); + panel_node = of_graph_get_remote_port_parent(endpoint); of_node_put(endpoint); - if (!panel) { + if (!panel_node) { dev_dbg(&pdev->dev, "no remote endpoint for port 1\n"); return -ENXIO; }
- lvds->panel = of_drm_find_panel(panel); - of_node_put(panel); - if (!lvds->panel) { + panel = of_drm_find_panel(panel_node); + of_node_put(panel_node); + if (!panel) { dev_dbg(&pdev->dev, "panel not found, deferring probe\n"); return -EPROBE_DEFER; }
- /* Register the bridge. */ - return drm_bridge_add(&lvds->bridge); + bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS); + if (IS_ERR(bridge)) + return PTR_ERR(bridge); + + platform_set_drvdata(pdev, bridge); + + return 0; }
static int lvds_encoder_remove(struct platform_device *pdev) { - struct lvds_encoder *encoder = platform_get_drvdata(pdev); + struct drm_bridge *bridge = platform_get_drvdata(pdev);
- drm_bridge_remove(&encoder->bridge); + drm_bridge_remove(bridge);
return 0; } diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c new file mode 100644 index 000000000000..fd1a78cb1205 --- /dev/null +++ b/drivers/gpu/drm/bridge/panel.c @@ -0,0 +1,197 @@ +/* + * Copyright (C) 2016 Laurent Pinchart laurent.pinchart@ideasonboard.com + * Copyright (C) 2017 Broadcom + * + * 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_panel.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_connector.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder.h> +#include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_panel.h> + +struct panel_bridge { + struct drm_bridge bridge; + struct drm_connector connector; + struct drm_panel *panel; + u32 connector_type; +}; + +static inline struct panel_bridge * +drm_bridge_to_panel_bridge(struct drm_bridge *bridge) +{ + return container_of(bridge, struct panel_bridge, bridge); +} + +static inline struct panel_bridge * +drm_connector_to_panel_bridge(struct drm_connector *connector) +{ + return container_of(connector, struct panel_bridge, connector); +} + +static int panel_bridge_connector_get_modes(struct drm_connector *connector) +{ + struct panel_bridge *panel_bridge = + drm_connector_to_panel_bridge(connector); + + return drm_panel_get_modes(panel_bridge->panel); +} + +static const struct drm_connector_helper_funcs panel_bridge_connector_helper_funcs = { + .get_modes = panel_bridge_connector_get_modes, +}; + +static const struct drm_connector_funcs panel_bridge_connector_funcs = { + .dpms = drm_atomic_helper_connector_dpms, + .reset = drm_atomic_helper_connector_reset, + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = drm_connector_cleanup, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static int panel_bridge_attach(struct drm_bridge *bridge) +{ + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); + struct drm_connector *connector = &panel_bridge->connector; + int ret; + + if (!bridge->encoder) { + DRM_ERROR("Missing encoder\n"); + return -ENODEV; + } + + drm_connector_helper_add(connector, + &panel_bridge_connector_helper_funcs); + + ret = drm_connector_init(bridge->dev, connector, + &panel_bridge_connector_funcs, + panel_bridge->connector_type); + if (ret) { + DRM_ERROR("Failed to initialize connector\n"); + return ret; + } + + drm_mode_connector_attach_encoder(&panel_bridge->connector, + bridge->encoder); + + ret = drm_panel_attach(panel_bridge->panel, &panel_bridge->connector); + if (ret < 0) + return ret; + + return 0; +} + +static void panel_bridge_detach(struct drm_bridge *bridge) +{ + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); + + drm_panel_detach(panel_bridge->panel); +} + +static void panel_bridge_pre_enable(struct drm_bridge *bridge) +{ + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); + + drm_panel_prepare(panel_bridge->panel); +} + +static void panel_bridge_enable(struct drm_bridge *bridge) +{ + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); + + drm_panel_enable(panel_bridge->panel); +} + +static void panel_bridge_disable(struct drm_bridge *bridge) +{ + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); + + drm_panel_disable(panel_bridge->panel); +} + +static void panel_bridge_post_disable(struct drm_bridge *bridge) +{ + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); + + drm_panel_unprepare(panel_bridge->panel); +} + +static const struct drm_bridge_funcs panel_bridge_bridge_funcs = { + .attach = panel_bridge_attach, + .detach = panel_bridge_detach, + .pre_enable = panel_bridge_pre_enable, + .enable = panel_bridge_enable, + .disable = panel_bridge_disable, + .post_disable = panel_bridge_post_disable, +}; + +/** + * drm_panel_bridge_add - Creates a drm_bridge and drm_connector that + * just calls the appropriate functions from drm_panel. + * + * @panel: The drm_panel being wrapped. Must be non-NULL. + * @connector_type: The DRM_MODE_CONNECTOR_* for the connector to be + * created. + * + * For drivers converting from directly using drm_panel: The expected + * usage pattern is that during either encoder module probe or DSI + * host attach, a drm_panel will be looked up through + * drm_of_find_panel_or_bridge(). drm_panel_bridge_add() is used to + * wrap that panel in the new bridge, and the result can then be + * passed to drm_bridge_attach(). The drm_panel_prepare() and related + * functions can be dropped from the encoder driver (they're now + * called by the KMS helpers before calling into the encoder), along + * with connector creation. When done with the bridge, + * drm_bridge_detach() should be called as normal, then + * drm_panel_bridge_remove() to free it. + */ +struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel, + u32 connector_type) +{ + struct panel_bridge *panel_bridge; + int ret; + + if (!panel) + return ERR_PTR(EINVAL); + + panel_bridge = devm_kzalloc(panel->dev, sizeof(*panel_bridge), + GFP_KERNEL); + if (!panel_bridge) + return ERR_PTR(-ENOMEM); + + panel_bridge->connector_type = connector_type; + panel_bridge->panel = panel; + + panel_bridge->bridge.funcs = &panel_bridge_bridge_funcs; + panel_bridge->bridge.of_node = panel->dev->of_node; + + ret = drm_bridge_add(&panel_bridge->bridge); + if (ret) + return ERR_PTR(ret); + + return &panel_bridge->bridge; +} +EXPORT_SYMBOL(drm_panel_bridge_add); + +/** + * drm_panel_bridge_remove - Unregisters and frees a drm_bridge + * created by drm_panel_bridge_add(). + * + * @bridge: The drm_bridge being freed. + */ +void drm_panel_bridge_remove(struct drm_bridge *bridge) +{ + struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge); + + drm_bridge_remove(bridge); + devm_kfree(panel_bridge->panel->dev, bridge); +} +EXPORT_SYMBOL(drm_panel_bridge_remove); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index fdd82fcbf168..bad2178ea3e0 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -29,6 +29,7 @@ #include <drm/drm_modes.h>
struct drm_bridge; +struct drm_panel;
/** * struct drm_bridge_funcs - drm_bridge control functions @@ -221,4 +222,10 @@ void drm_bridge_mode_set(struct drm_bridge *bridge, void drm_bridge_pre_enable(struct drm_bridge *bridge); void drm_bridge_enable(struct drm_bridge *bridge);
+#ifdef CONFIG_DRM_PANEL +struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel, + u32 connector_type); +void drm_panel_bridge_remove(struct drm_bridge *bridge); +#endif + #endif
The newer version of the RPi panel driver is going to be a combination of a bridge and a panel, but we should also support panels without a bridge, so the panel-bridge layer lets us do that cleanly.
v2: Drop "dev" argument.
Signed-off-by: Eric Anholt eric@anholt.net Acked-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/vc4/Kconfig | 2 +- drivers/gpu/drm/vc4/vc4_dsi.c | 154 ++++++------------------------------------ 2 files changed, 21 insertions(+), 135 deletions(-)
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig index b16aefe4a8d3..4361bdcfd28a 100644 --- a/drivers/gpu/drm/vc4/Kconfig +++ b/drivers/gpu/drm/vc4/Kconfig @@ -7,7 +7,7 @@ config DRM_VC4 select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER select DRM_GEM_CMA_HELPER - select DRM_PANEL + select DRM_PANEL_BRIDGE select SND_PCM select SND_PCM_ELD select SND_SOC_GENERIC_DMAENGINE_PCM diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index deba62008fd0..fb54a9d10360 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -503,8 +503,8 @@ struct vc4_dsi {
struct mipi_dsi_host dsi_host; struct drm_encoder *encoder; - struct drm_connector *connector; - struct drm_panel *panel; + struct drm_bridge *bridge; + bool is_panel_bridge;
void __iomem *regs;
@@ -604,18 +604,6 @@ to_vc4_dsi_encoder(struct drm_encoder *encoder) return container_of(encoder, struct vc4_dsi_encoder, base.base); }
-/* VC4 DSI connector KMS struct */ -struct vc4_dsi_connector { - struct drm_connector base; - struct vc4_dsi *dsi; -}; - -static inline struct vc4_dsi_connector * -to_vc4_dsi_connector(struct drm_connector *connector) -{ - return container_of(connector, struct vc4_dsi_connector, base); -} - #define DSI_REG(reg) { reg, #reg } static const struct { u32 reg; @@ -723,79 +711,6 @@ int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused) } #endif
-static enum drm_connector_status -vc4_dsi_connector_detect(struct drm_connector *connector, bool force) -{ - struct vc4_dsi_connector *vc4_connector = - to_vc4_dsi_connector(connector); - struct vc4_dsi *dsi = vc4_connector->dsi; - - if (dsi->panel) - return connector_status_connected; - else - return connector_status_disconnected; -} - -static void vc4_dsi_connector_destroy(struct drm_connector *connector) -{ - drm_connector_unregister(connector); - drm_connector_cleanup(connector); -} - -static int vc4_dsi_connector_get_modes(struct drm_connector *connector) -{ - struct vc4_dsi_connector *vc4_connector = - to_vc4_dsi_connector(connector); - struct vc4_dsi *dsi = vc4_connector->dsi; - - if (dsi->panel) - return drm_panel_get_modes(dsi->panel); - - return 0; -} - -static const struct drm_connector_funcs vc4_dsi_connector_funcs = { - .dpms = drm_atomic_helper_connector_dpms, - .detect = vc4_dsi_connector_detect, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = vc4_dsi_connector_destroy, - .reset = drm_atomic_helper_connector_reset, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -static const struct drm_connector_helper_funcs vc4_dsi_connector_helper_funcs = { - .get_modes = vc4_dsi_connector_get_modes, -}; - -static struct drm_connector *vc4_dsi_connector_init(struct drm_device *dev, - struct vc4_dsi *dsi) -{ - struct drm_connector *connector; - struct vc4_dsi_connector *dsi_connector; - - dsi_connector = devm_kzalloc(dev->dev, sizeof(*dsi_connector), - GFP_KERNEL); - if (!dsi_connector) - return ERR_PTR(-ENOMEM); - - connector = &dsi_connector->base; - - dsi_connector->dsi = dsi; - - drm_connector_init(dev, connector, &vc4_dsi_connector_funcs, - DRM_MODE_CONNECTOR_DSI); - drm_connector_helper_add(connector, &vc4_dsi_connector_helper_funcs); - - connector->polled = 0; - connector->interlace_allowed = 0; - connector->doublescan_allowed = 0; - - drm_mode_connector_attach_encoder(connector, dsi->encoder); - - return connector; -} - static void vc4_dsi_encoder_destroy(struct drm_encoder *encoder) { drm_encoder_cleanup(encoder); @@ -893,12 +808,8 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder) struct vc4_dsi *dsi = vc4_encoder->dsi; struct device *dev = &dsi->pdev->dev;
- drm_panel_disable(dsi->panel); - vc4_dsi_ulps(dsi, true);
- drm_panel_unprepare(dsi->panel); - clk_disable_unprepare(dsi->pll_phy_clock); clk_disable_unprepare(dsi->escape_clock); clk_disable_unprepare(dsi->pixel_clock); @@ -929,12 +840,6 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) return; }
- ret = drm_panel_prepare(dsi->panel); - if (ret) { - DRM_ERROR("Panel failed to prepare\n"); - return; - } - if (debug_dump_regs) { DRM_INFO("DSI regs before:\n"); vc4_dsi_dump_regs(dsi); @@ -1184,13 +1089,6 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) DRM_INFO("DSI regs after:\n"); vc4_dsi_dump_regs(dsi); } - - ret = drm_panel_enable(dsi->panel); - if (ret) { - DRM_ERROR("Panel failed to enable\n"); - drm_panel_unprepare(dsi->panel); - return; - } }
static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host, @@ -1366,17 +1264,22 @@ static int vc4_dsi_host_attach(struct mipi_dsi_host *host, return 0; }
- dsi->panel = of_drm_find_panel(device->dev.of_node); - if (!dsi->panel) - return 0; - - ret = drm_panel_attach(dsi->panel, dsi->connector); - if (ret != 0) - return ret; + dsi->bridge = of_drm_find_bridge(device->dev.of_node); + if (!dsi->bridge) { + struct drm_panel *panel = + of_drm_find_panel(device->dev.of_node);
- drm_helper_hpd_irq_event(dsi->connector->dev); + dsi->bridge = drm_panel_bridge_add(panel, + DRM_MODE_CONNECTOR_DSI); + if (IS_ERR(dsi->bridge)) { + ret = PTR_ERR(dsi->bridge); + dsi->bridge = NULL; + return ret; + } + dsi->is_panel_bridge = true; + }
- return 0; + return drm_bridge_attach(dsi->encoder, dsi->bridge, NULL); }
static int vc4_dsi_host_detach(struct mipi_dsi_host *host, @@ -1384,15 +1287,9 @@ static int vc4_dsi_host_detach(struct mipi_dsi_host *host, { struct vc4_dsi *dsi = host_to_dsi(host);
- if (dsi->panel) { - int ret = drm_panel_detach(dsi->panel); - - if (ret) - return ret; - - dsi->panel = NULL; - - drm_helper_hpd_irq_event(dsi->connector->dev); + if (dsi->is_panel_bridge) { + drm_panel_bridge_remove(dsi->bridge); + dsi->bridge = NULL; }
return 0; @@ -1658,12 +1555,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) DRM_MODE_ENCODER_DSI, NULL); drm_encoder_helper_add(dsi->encoder, &vc4_dsi_encoder_helper_funcs);
- dsi->connector = vc4_dsi_connector_init(drm, dsi); - if (IS_ERR(dsi->connector)) { - ret = PTR_ERR(dsi->connector); - goto err_destroy_encoder; - } - dsi->dsi_host.ops = &vc4_dsi_host_ops; dsi->dsi_host.dev = dev;
@@ -1674,11 +1565,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) pm_runtime_enable(dev);
return 0; - -err_destroy_encoder: - vc4_dsi_encoder_destroy(dsi->encoder); - - return ret; }
static void vc4_dsi_unbind(struct device *dev, struct device *master, @@ -1690,7 +1576,7 @@ static void vc4_dsi_unbind(struct device *dev, struct device *master,
pm_runtime_disable(dev);
- vc4_dsi_connector_destroy(dsi->connector); + drm_bridge_remove(dsi->bridge); vc4_dsi_encoder_destroy(dsi->encoder);
mipi_dsi_host_unregister(&dsi->dsi_host);
On Thu, 11 May 2017 11:31:23 -0700 Eric Anholt eric@anholt.net wrote:
The newer version of the RPi panel driver is going to be a combination of a bridge and a panel, but we should also support panels without a bridge, so the panel-bridge layer lets us do that cleanly.
v2: Drop "dev" argument.
Signed-off-by: Eric Anholt eric@anholt.net Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
Reviewed-by: Boris Brezillon boris.brezillon@free-electrons.com
drivers/gpu/drm/vc4/Kconfig | 2 +- drivers/gpu/drm/vc4/vc4_dsi.c | 154 ++++++------------------------------------ 2 files changed, 21 insertions(+), 135 deletions(-)
diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig index b16aefe4a8d3..4361bdcfd28a 100644 --- a/drivers/gpu/drm/vc4/Kconfig +++ b/drivers/gpu/drm/vc4/Kconfig @@ -7,7 +7,7 @@ config DRM_VC4 select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER select DRM_GEM_CMA_HELPER
- select DRM_PANEL
- select DRM_PANEL_BRIDGE select SND_PCM select SND_PCM_ELD select SND_SOC_GENERIC_DMAENGINE_PCM
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index deba62008fd0..fb54a9d10360 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -503,8 +503,8 @@ struct vc4_dsi {
struct mipi_dsi_host dsi_host; struct drm_encoder *encoder;
- struct drm_connector *connector;
- struct drm_panel *panel;
struct drm_bridge *bridge;
bool is_panel_bridge;
void __iomem *regs;
@@ -604,18 +604,6 @@ to_vc4_dsi_encoder(struct drm_encoder *encoder) return container_of(encoder, struct vc4_dsi_encoder, base.base); }
-/* VC4 DSI connector KMS struct */ -struct vc4_dsi_connector {
- struct drm_connector base;
- struct vc4_dsi *dsi;
-};
-static inline struct vc4_dsi_connector * -to_vc4_dsi_connector(struct drm_connector *connector) -{
- return container_of(connector, struct vc4_dsi_connector, base);
-}
#define DSI_REG(reg) { reg, #reg } static const struct { u32 reg; @@ -723,79 +711,6 @@ int vc4_dsi_debugfs_regs(struct seq_file *m, void *unused) } #endif
-static enum drm_connector_status -vc4_dsi_connector_detect(struct drm_connector *connector, bool force) -{
- struct vc4_dsi_connector *vc4_connector =
to_vc4_dsi_connector(connector);- struct vc4_dsi *dsi = vc4_connector->dsi;
- if (dsi->panel)
return connector_status_connected;- else
return connector_status_disconnected;-}
-static void vc4_dsi_connector_destroy(struct drm_connector *connector) -{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-static int vc4_dsi_connector_get_modes(struct drm_connector *connector) -{
- struct vc4_dsi_connector *vc4_connector =
to_vc4_dsi_connector(connector);- struct vc4_dsi *dsi = vc4_connector->dsi;
- if (dsi->panel)
return drm_panel_get_modes(dsi->panel);- return 0;
-}
-static const struct drm_connector_funcs vc4_dsi_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .detect = vc4_dsi_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = vc4_dsi_connector_destroy,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-static const struct drm_connector_helper_funcs vc4_dsi_connector_helper_funcs = {
- .get_modes = vc4_dsi_connector_get_modes,
-};
-static struct drm_connector *vc4_dsi_connector_init(struct drm_device *dev,
struct vc4_dsi *dsi)-{
- struct drm_connector *connector;
- struct vc4_dsi_connector *dsi_connector;
- dsi_connector = devm_kzalloc(dev->dev, sizeof(*dsi_connector),
GFP_KERNEL);- if (!dsi_connector)
return ERR_PTR(-ENOMEM);- connector = &dsi_connector->base;
- dsi_connector->dsi = dsi;
- drm_connector_init(dev, connector, &vc4_dsi_connector_funcs,
DRM_MODE_CONNECTOR_DSI);- drm_connector_helper_add(connector, &vc4_dsi_connector_helper_funcs);
- connector->polled = 0;
- connector->interlace_allowed = 0;
- connector->doublescan_allowed = 0;
- drm_mode_connector_attach_encoder(connector, dsi->encoder);
- return connector;
-}
static void vc4_dsi_encoder_destroy(struct drm_encoder *encoder) { drm_encoder_cleanup(encoder); @@ -893,12 +808,8 @@ static void vc4_dsi_encoder_disable(struct drm_encoder *encoder) struct vc4_dsi *dsi = vc4_encoder->dsi; struct device *dev = &dsi->pdev->dev;
drm_panel_disable(dsi->panel);
vc4_dsi_ulps(dsi, true);
drm_panel_unprepare(dsi->panel);
clk_disable_unprepare(dsi->pll_phy_clock); clk_disable_unprepare(dsi->escape_clock); clk_disable_unprepare(dsi->pixel_clock);
@@ -929,12 +840,6 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) return; }
- ret = drm_panel_prepare(dsi->panel);
- if (ret) {
DRM_ERROR("Panel failed to prepare\n");return;- }
- if (debug_dump_regs) { DRM_INFO("DSI regs before:\n"); vc4_dsi_dump_regs(dsi);
@@ -1184,13 +1089,6 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) DRM_INFO("DSI regs after:\n"); vc4_dsi_dump_regs(dsi); }
- ret = drm_panel_enable(dsi->panel);
- if (ret) {
DRM_ERROR("Panel failed to enable\n");drm_panel_unprepare(dsi->panel);return;- }
}
static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host, @@ -1366,17 +1264,22 @@ static int vc4_dsi_host_attach(struct mipi_dsi_host *host, return 0; }
- dsi->panel = of_drm_find_panel(device->dev.of_node);
- if (!dsi->panel)
return 0;- ret = drm_panel_attach(dsi->panel, dsi->connector);
- if (ret != 0)
return ret;
- dsi->bridge = of_drm_find_bridge(device->dev.of_node);
- if (!dsi->bridge) {
struct drm_panel *panel =of_drm_find_panel(device->dev.of_node);
- drm_helper_hpd_irq_event(dsi->connector->dev);
dsi->bridge = drm_panel_bridge_add(panel,DRM_MODE_CONNECTOR_DSI);if (IS_ERR(dsi->bridge)) {ret = PTR_ERR(dsi->bridge);dsi->bridge = NULL;return ret;}dsi->is_panel_bridge = true;- }
- return 0;
- return drm_bridge_attach(dsi->encoder, dsi->bridge, NULL);
}
static int vc4_dsi_host_detach(struct mipi_dsi_host *host, @@ -1384,15 +1287,9 @@ static int vc4_dsi_host_detach(struct mipi_dsi_host *host, { struct vc4_dsi *dsi = host_to_dsi(host);
- if (dsi->panel) {
int ret = drm_panel_detach(dsi->panel);if (ret)return ret;dsi->panel = NULL;drm_helper_hpd_irq_event(dsi->connector->dev);
if (dsi->is_panel_bridge) {
drm_panel_bridge_remove(dsi->bridge);dsi->bridge = NULL;}
return 0;
@@ -1658,12 +1555,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) DRM_MODE_ENCODER_DSI, NULL); drm_encoder_helper_add(dsi->encoder, &vc4_dsi_encoder_helper_funcs);
- dsi->connector = vc4_dsi_connector_init(drm, dsi);
- if (IS_ERR(dsi->connector)) {
ret = PTR_ERR(dsi->connector);goto err_destroy_encoder;- }
- dsi->dsi_host.ops = &vc4_dsi_host_ops; dsi->dsi_host.dev = dev;
@@ -1674,11 +1565,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) pm_runtime_enable(dev);
return 0;
-err_destroy_encoder:
- vc4_dsi_encoder_destroy(dsi->encoder);
- return ret;
}
static void vc4_dsi_unbind(struct device *dev, struct device *master, @@ -1690,7 +1576,7 @@ static void vc4_dsi_unbind(struct device *dev, struct device *master,
pm_runtime_disable(dev);
- vc4_dsi_connector_destroy(dsi->connector);
drm_bridge_remove(dsi->bridge); vc4_dsi_encoder_destroy(dsi->encoder);
mipi_dsi_host_unregister(&dsi->dsi_host);
Another 100 lines of boilerplate gone, while allowing for bridges to be connected in the display chain.
Signed-off-by: Eric Anholt eric@anholt.net --- drivers/gpu/drm/vc4/vc4_dpi.c | 164 ++++++++---------------------------------- 1 file changed, 30 insertions(+), 134 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index c6d703903fd9..98532e2cd2b7 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -23,8 +23,10 @@ */
#include "drm_atomic_helper.h" +#include "drm_bridge.h" #include "drm_crtc_helper.h" #include "drm_edid.h" +#include "drm_of.h" #include "drm_panel.h" #include "linux/clk.h" #include "linux/component.h" @@ -95,7 +97,8 @@ struct vc4_dpi {
struct drm_encoder *encoder; struct drm_connector *connector; - struct drm_panel *panel; + struct drm_bridge *bridge; + bool is_panel_bridge;
void __iomem *regs;
@@ -118,24 +121,6 @@ to_vc4_dpi_encoder(struct drm_encoder *encoder) return container_of(encoder, struct vc4_dpi_encoder, base.base); }
-/* VC4 DPI connector KMS struct */ -struct vc4_dpi_connector { - struct drm_connector base; - struct vc4_dpi *dpi; - - /* Since the connector is attached to just the one encoder, - * this is the reference to it so we can do the best_encoder() - * hook. - */ - struct drm_encoder *encoder; -}; - -static inline struct vc4_dpi_connector * -to_vc4_dpi_connector(struct drm_connector *connector) -{ - return container_of(connector, struct vc4_dpi_connector, base); -} - #define DPI_REG(reg) { reg, #reg } static const struct { u32 reg; @@ -167,80 +152,6 @@ int vc4_dpi_debugfs_regs(struct seq_file *m, void *unused) } #endif
-static enum drm_connector_status -vc4_dpi_connector_detect(struct drm_connector *connector, bool force) -{ - struct vc4_dpi_connector *vc4_connector = - to_vc4_dpi_connector(connector); - struct vc4_dpi *dpi = vc4_connector->dpi; - - if (dpi->panel) - return connector_status_connected; - else - return connector_status_disconnected; -} - -static void vc4_dpi_connector_destroy(struct drm_connector *connector) -{ - drm_connector_unregister(connector); - drm_connector_cleanup(connector); -} - -static int vc4_dpi_connector_get_modes(struct drm_connector *connector) -{ - struct vc4_dpi_connector *vc4_connector = - to_vc4_dpi_connector(connector); - struct vc4_dpi *dpi = vc4_connector->dpi; - - if (dpi->panel) - return drm_panel_get_modes(dpi->panel); - - return 0; -} - -static const struct drm_connector_funcs vc4_dpi_connector_funcs = { - .dpms = drm_atomic_helper_connector_dpms, - .detect = vc4_dpi_connector_detect, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = vc4_dpi_connector_destroy, - .reset = drm_atomic_helper_connector_reset, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -static const struct drm_connector_helper_funcs vc4_dpi_connector_helper_funcs = { - .get_modes = vc4_dpi_connector_get_modes, -}; - -static struct drm_connector *vc4_dpi_connector_init(struct drm_device *dev, - struct vc4_dpi *dpi) -{ - struct drm_connector *connector = NULL; - struct vc4_dpi_connector *dpi_connector; - - dpi_connector = devm_kzalloc(dev->dev, sizeof(*dpi_connector), - GFP_KERNEL); - if (!dpi_connector) - return ERR_PTR(-ENOMEM); - - connector = &dpi_connector->base; - - dpi_connector->encoder = dpi->encoder; - dpi_connector->dpi = dpi; - - drm_connector_init(dev, connector, &vc4_dpi_connector_funcs, - DRM_MODE_CONNECTOR_DPI); - drm_connector_helper_add(connector, &vc4_dpi_connector_helper_funcs); - - connector->polled = 0; - connector->interlace_allowed = 0; - connector->doublescan_allowed = 0; - - drm_mode_connector_attach_encoder(connector, dpi->encoder); - - return connector; -} - static const struct drm_encoder_funcs vc4_dpi_encoder_funcs = { .destroy = drm_encoder_cleanup, }; @@ -250,11 +161,7 @@ static void vc4_dpi_encoder_disable(struct drm_encoder *encoder) struct vc4_dpi_encoder *vc4_encoder = to_vc4_dpi_encoder(encoder); struct vc4_dpi *dpi = vc4_encoder->dpi;
- drm_panel_disable(dpi->panel); - clk_disable_unprepare(dpi->pixel_clock); - - drm_panel_unprepare(dpi->panel); }
static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) @@ -265,12 +172,6 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) u32 dpi_c = DPI_ENABLE | DPI_OUTPUT_ENABLE_MODE; int ret;
- ret = drm_panel_prepare(dpi->panel); - if (ret) { - DRM_ERROR("Panel failed to prepare\n"); - return; - } - if (dpi->connector->display_info.num_bus_formats) { u32 bus_format = dpi->connector->display_info.bus_formats[0];
@@ -321,13 +222,6 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) ret = clk_prepare_enable(dpi->pixel_clock); if (ret) DRM_ERROR("Failed to set clock rate: %d\n", ret); - - ret = drm_panel_enable(dpi->panel); - if (ret) { - DRM_ERROR("Panel failed to enable\n"); - drm_panel_unprepare(dpi->panel); - return; - } }
static bool vc4_dpi_encoder_mode_fixup(struct drm_encoder *encoder, @@ -351,24 +245,34 @@ static const struct of_device_id vc4_dpi_dt_match[] = { {} };
-/* Walks the OF graph to find the panel node and then asks DRM to look - * up the panel. +/* Sets up the next link in the display chain, whether it's a panel or + * a bridge. */ -static struct drm_panel *vc4_dpi_get_panel(struct device *dev) +static int vc4_dpi_init_bridge(struct vc4_dpi *dpi) { - struct device_node *panel_node; - struct device_node *np = dev->of_node; + struct device *dev = &dpi->pdev->dev; struct drm_panel *panel; + int ret;
- /* don't proceed if we have an endpoint but no panel_node tied to it */ - panel_node = of_graph_get_remote_node(np, 0, 0); - if (!panel_node) - return NULL; + ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, + &panel, &dpi->bridge); + if (ret) { + /* If nothing was connected in the DT, that's not an + * error. + */ + if (ret == -ENODEV) + return 0; + else + return ret; + }
- panel = of_drm_find_panel(panel_node); - of_node_put(panel_node); + if (panel) { + dpi->bridge = drm_panel_bridge_add(panel, + DRM_MODE_CONNECTOR_DPI); + dpi->is_panel_bridge = true; + }
- return panel; + return drm_bridge_attach(dpi->encoder, dpi->bridge, NULL); }
static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) @@ -422,20 +326,13 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) if (ret) DRM_ERROR("Failed to turn on core clock: %d\n", ret);
- dpi->panel = vc4_dpi_get_panel(dev); - drm_encoder_init(drm, dpi->encoder, &vc4_dpi_encoder_funcs, DRM_MODE_ENCODER_DPI, NULL); drm_encoder_helper_add(dpi->encoder, &vc4_dpi_encoder_helper_funcs);
- dpi->connector = vc4_dpi_connector_init(drm, dpi); - if (IS_ERR(dpi->connector)) { - ret = PTR_ERR(dpi->connector); + ret = vc4_dpi_init_bridge(dpi); + if (ret) goto err_destroy_encoder; - } - - if (dpi->panel) - drm_panel_attach(dpi->panel, dpi->connector);
dev_set_drvdata(dev, dpi);
@@ -456,10 +353,9 @@ static void vc4_dpi_unbind(struct device *dev, struct device *master, struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_dpi *dpi = dev_get_drvdata(dev);
- if (dpi->panel) - drm_panel_detach(dpi->panel); + if (dpi->is_panel_bridge) + drm_panel_bridge_remove(dpi->bridge);
- vc4_dpi_connector_destroy(dpi->connector); drm_encoder_cleanup(dpi->encoder);
clk_disable_unprepare(dpi->core_clock);
On Thu, 11 May 2017 11:31:24 -0700 Eric Anholt eric@anholt.net wrote:
Another 100 lines of boilerplate gone, while allowing for bridges to be connected in the display chain.
Signed-off-by: Eric Anholt eric@anholt.net
Reviewed-by: Boris Brezillon boris.brezillon@free-electrons.com
drivers/gpu/drm/vc4/vc4_dpi.c | 164 ++++++++---------------------------------- 1 file changed, 30 insertions(+), 134 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c index c6d703903fd9..98532e2cd2b7 100644 --- a/drivers/gpu/drm/vc4/vc4_dpi.c +++ b/drivers/gpu/drm/vc4/vc4_dpi.c @@ -23,8 +23,10 @@ */
#include "drm_atomic_helper.h" +#include "drm_bridge.h" #include "drm_crtc_helper.h" #include "drm_edid.h" +#include "drm_of.h" #include "drm_panel.h" #include "linux/clk.h" #include "linux/component.h" @@ -95,7 +97,8 @@ struct vc4_dpi {
struct drm_encoder *encoder; struct drm_connector *connector;
- struct drm_panel *panel;
struct drm_bridge *bridge;
bool is_panel_bridge;
void __iomem *regs;
@@ -118,24 +121,6 @@ to_vc4_dpi_encoder(struct drm_encoder *encoder) return container_of(encoder, struct vc4_dpi_encoder, base.base); }
-/* VC4 DPI connector KMS struct */ -struct vc4_dpi_connector {
- struct drm_connector base;
- struct vc4_dpi *dpi;
- /* Since the connector is attached to just the one encoder,
* this is the reference to it so we can do the best_encoder()* hook.*/- struct drm_encoder *encoder;
-};
-static inline struct vc4_dpi_connector * -to_vc4_dpi_connector(struct drm_connector *connector) -{
- return container_of(connector, struct vc4_dpi_connector, base);
-}
#define DPI_REG(reg) { reg, #reg } static const struct { u32 reg; @@ -167,80 +152,6 @@ int vc4_dpi_debugfs_regs(struct seq_file *m, void *unused) } #endif
-static enum drm_connector_status -vc4_dpi_connector_detect(struct drm_connector *connector, bool force) -{
- struct vc4_dpi_connector *vc4_connector =
to_vc4_dpi_connector(connector);- struct vc4_dpi *dpi = vc4_connector->dpi;
- if (dpi->panel)
return connector_status_connected;- else
return connector_status_disconnected;-}
-static void vc4_dpi_connector_destroy(struct drm_connector *connector) -{
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-static int vc4_dpi_connector_get_modes(struct drm_connector *connector) -{
- struct vc4_dpi_connector *vc4_connector =
to_vc4_dpi_connector(connector);- struct vc4_dpi *dpi = vc4_connector->dpi;
- if (dpi->panel)
return drm_panel_get_modes(dpi->panel);- return 0;
-}
-static const struct drm_connector_funcs vc4_dpi_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .detect = vc4_dpi_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = vc4_dpi_connector_destroy,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-static const struct drm_connector_helper_funcs vc4_dpi_connector_helper_funcs = {
- .get_modes = vc4_dpi_connector_get_modes,
-};
-static struct drm_connector *vc4_dpi_connector_init(struct drm_device *dev,
struct vc4_dpi *dpi)-{
- struct drm_connector *connector = NULL;
- struct vc4_dpi_connector *dpi_connector;
- dpi_connector = devm_kzalloc(dev->dev, sizeof(*dpi_connector),
GFP_KERNEL);- if (!dpi_connector)
return ERR_PTR(-ENOMEM);- connector = &dpi_connector->base;
- dpi_connector->encoder = dpi->encoder;
- dpi_connector->dpi = dpi;
- drm_connector_init(dev, connector, &vc4_dpi_connector_funcs,
DRM_MODE_CONNECTOR_DPI);- drm_connector_helper_add(connector, &vc4_dpi_connector_helper_funcs);
- connector->polled = 0;
- connector->interlace_allowed = 0;
- connector->doublescan_allowed = 0;
- drm_mode_connector_attach_encoder(connector, dpi->encoder);
- return connector;
-}
static const struct drm_encoder_funcs vc4_dpi_encoder_funcs = { .destroy = drm_encoder_cleanup, }; @@ -250,11 +161,7 @@ static void vc4_dpi_encoder_disable(struct drm_encoder *encoder) struct vc4_dpi_encoder *vc4_encoder = to_vc4_dpi_encoder(encoder); struct vc4_dpi *dpi = vc4_encoder->dpi;
- drm_panel_disable(dpi->panel);
- clk_disable_unprepare(dpi->pixel_clock);
- drm_panel_unprepare(dpi->panel);
}
static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) @@ -265,12 +172,6 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) u32 dpi_c = DPI_ENABLE | DPI_OUTPUT_ENABLE_MODE; int ret;
- ret = drm_panel_prepare(dpi->panel);
- if (ret) {
DRM_ERROR("Panel failed to prepare\n");return;- }
- if (dpi->connector->display_info.num_bus_formats) { u32 bus_format = dpi->connector->display_info.bus_formats[0];
@@ -321,13 +222,6 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder) ret = clk_prepare_enable(dpi->pixel_clock); if (ret) DRM_ERROR("Failed to set clock rate: %d\n", ret);
- ret = drm_panel_enable(dpi->panel);
- if (ret) {
DRM_ERROR("Panel failed to enable\n");drm_panel_unprepare(dpi->panel);return;- }
}
static bool vc4_dpi_encoder_mode_fixup(struct drm_encoder *encoder, @@ -351,24 +245,34 @@ static const struct of_device_id vc4_dpi_dt_match[] = { {} };
-/* Walks the OF graph to find the panel node and then asks DRM to look
- up the panel.
+/* Sets up the next link in the display chain, whether it's a panel or
*/
- a bridge.
-static struct drm_panel *vc4_dpi_get_panel(struct device *dev) +static int vc4_dpi_init_bridge(struct vc4_dpi *dpi) {
- struct device_node *panel_node;
- struct device_node *np = dev->of_node;
- struct device *dev = &dpi->pdev->dev; struct drm_panel *panel;
- int ret;
- /* don't proceed if we have an endpoint but no panel_node tied to it */
- panel_node = of_graph_get_remote_node(np, 0, 0);
- if (!panel_node)
return NULL;
- ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
&panel, &dpi->bridge);- if (ret) {
/* If nothing was connected in the DT, that's not an* error.*/if (ret == -ENODEV)return 0;elsereturn ret;- }
- panel = of_drm_find_panel(panel_node);
- of_node_put(panel_node);
- if (panel) {
dpi->bridge = drm_panel_bridge_add(panel,DRM_MODE_CONNECTOR_DPI);dpi->is_panel_bridge = true;- }
- return panel;
- return drm_bridge_attach(dpi->encoder, dpi->bridge, NULL);
}
static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) @@ -422,20 +326,13 @@ static int vc4_dpi_bind(struct device *dev, struct device *master, void *data) if (ret) DRM_ERROR("Failed to turn on core clock: %d\n", ret);
dpi->panel = vc4_dpi_get_panel(dev);
drm_encoder_init(drm, dpi->encoder, &vc4_dpi_encoder_funcs, DRM_MODE_ENCODER_DPI, NULL); drm_encoder_helper_add(dpi->encoder, &vc4_dpi_encoder_helper_funcs);
dpi->connector = vc4_dpi_connector_init(drm, dpi);
if (IS_ERR(dpi->connector)) {
ret = PTR_ERR(dpi->connector);
- ret = vc4_dpi_init_bridge(dpi);
- if (ret) goto err_destroy_encoder;
}
if (dpi->panel)
drm_panel_attach(dpi->panel, dpi->connector);dev_set_drvdata(dev, dpi);
@@ -456,10 +353,9 @@ static void vc4_dpi_unbind(struct device *dev, struct device *master, struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_dpi *dpi = dev_get_drvdata(dev);
- if (dpi->panel)
drm_panel_detach(dpi->panel);
- if (dpi->is_panel_bridge)
drm_panel_bridge_remove(dpi->bridge);
vc4_dpi_connector_destroy(dpi->connector); drm_encoder_cleanup(dpi->encoder);
clk_disable_unprepare(dpi->core_clock);
Avoids a bunch of connector boilerplate. Note that this causes panel prepare() to be moved before mtk_dsi_poweron() and unprepare() to be after poweroff(). I think this is the expected usage of the panel API (enable should be when you do things that require the link to be brought up), but there may be issues here.
Signed-off-by: Eric Anholt eric@anholt.net ---
Note that I haven't tested this change, and am not committed to this patch. It's just an optional cleanup, if it works for you.
drivers/gpu/drm/mediatek/mtk_dsi.c | 125 ++++--------------------------------- 1 file changed, 13 insertions(+), 112 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 808b995a990f..764bd8b9c256 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -160,7 +160,6 @@ struct mtk_dsi { struct device *dev; struct mipi_dsi_host host; struct drm_encoder encoder; - struct drm_connector conn; struct drm_panel *panel; struct drm_bridge *bridge; struct phy *phy; @@ -188,11 +187,6 @@ static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e) return container_of(e, struct mtk_dsi, encoder); }
-static inline struct mtk_dsi *connector_to_dsi(struct drm_connector *c) -{ - return container_of(c, struct mtk_dsi, conn); -} - static inline struct mtk_dsi *host_to_dsi(struct mipi_dsi_host *h) { return container_of(h, struct mtk_dsi, host); @@ -603,16 +597,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) mtk_dsi_lane0_ulp_mode_leave(dsi); mtk_dsi_clk_hs_mode(dsi, 0);
- if (dsi->panel) { - if (drm_panel_prepare(dsi->panel)) { - DRM_ERROR("failed to prepare the panel\n"); - goto err_disable_digital_clk; - } - } - return 0; -err_disable_digital_clk: - clk_disable_unprepare(dsi->digital_clk); err_disable_engine_clk: clk_disable_unprepare(dsi->engine_clk); err_phy_power_off: @@ -630,15 +615,7 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) if (--dsi->refcount != 0) return;
- if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) { - if (dsi->panel) { - if (drm_panel_unprepare(dsi->panel)) { - DRM_ERROR("failed to unprepare the panel\n"); - return; - } - } - } - + mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); mtk_dsi_reset_engine(dsi); mtk_dsi_lane0_ulp_mode_enter(dsi); mtk_dsi_clk_ulp_mode_enter(dsi); @@ -669,19 +646,9 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
mtk_dsi_start(dsi);
- if (dsi->panel) { - if (drm_panel_enable(dsi->panel)) { - DRM_ERROR("failed to enable the panel\n"); - goto err_dsi_power_off; - } - } - dsi->enabled = true;
return; -err_dsi_power_off: - mtk_dsi_stop(dsi); - mtk_dsi_poweroff(dsi); }
static void mtk_output_dsi_disable(struct mtk_dsi *dsi) @@ -689,13 +656,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) if (!dsi->enabled) return;
- if (dsi->panel) { - if (drm_panel_disable(dsi->panel)) { - DRM_ERROR("failed to disable the panel\n"); - return; - } - } - mtk_dsi_stop(dsi); mtk_dsi_poweroff(dsi);
@@ -750,13 +710,6 @@ static void mtk_dsi_encoder_enable(struct drm_encoder *encoder) mtk_output_dsi_enable(dsi); }
-static int mtk_dsi_connector_get_modes(struct drm_connector *connector) -{ - struct mtk_dsi *dsi = connector_to_dsi(connector); - - return drm_panel_get_modes(dsi->panel); -} - static const struct drm_encoder_helper_funcs mtk_dsi_encoder_helper_funcs = { .mode_fixup = mtk_dsi_encoder_mode_fixup, .mode_set = mtk_dsi_encoder_mode_set, @@ -764,52 +717,7 @@ static const struct drm_encoder_helper_funcs mtk_dsi_encoder_helper_funcs = { .enable = mtk_dsi_encoder_enable, };
-static const struct drm_connector_funcs mtk_dsi_connector_funcs = { - .dpms = drm_atomic_helper_connector_dpms, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = drm_connector_cleanup, - .reset = drm_atomic_helper_connector_reset, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -static const struct drm_connector_helper_funcs - mtk_dsi_connector_helper_funcs = { - .get_modes = mtk_dsi_connector_get_modes, -}; - -static int mtk_dsi_create_connector(struct drm_device *drm, struct mtk_dsi *dsi) -{ - int ret; - - ret = drm_connector_init(drm, &dsi->conn, &mtk_dsi_connector_funcs, - DRM_MODE_CONNECTOR_DSI); - if (ret) { - DRM_ERROR("Failed to connector init to drm\n"); - return ret; - } - - drm_connector_helper_add(&dsi->conn, &mtk_dsi_connector_helper_funcs); - - dsi->conn.dpms = DRM_MODE_DPMS_OFF; - drm_mode_connector_attach_encoder(&dsi->conn, &dsi->encoder); - - if (dsi->panel) { - ret = drm_panel_attach(dsi->panel, &dsi->conn); - if (ret) { - DRM_ERROR("Failed to attach panel to drm\n"); - goto err_connector_cleanup; - } - } - - return 0; - -err_connector_cleanup: - drm_connector_cleanup(&dsi->conn); - return ret; -} - -static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) +static int mtk_dsi_create_enc(struct drm_device *drm, struct mtk_dsi *dsi) { int ret;
@@ -827,15 +735,10 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) */ dsi->encoder.possible_crtcs = 1;
- /* If there's a bridge, attach to it and let it create the connector */ ret = drm_bridge_attach(&dsi->encoder, dsi->bridge, NULL); if (ret) { DRM_ERROR("Failed to attach bridge to drm\n"); - - /* Otherwise create our own connector and attach to a panel */ - ret = mtk_dsi_create_connector(drm, dsi); - if (ret) - goto err_encoder_cleanup; + goto err_encoder_cleanup; }
return 0; @@ -848,9 +751,8 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi) { drm_encoder_cleanup(&dsi->encoder); - /* Skip connector cleanup if creation was delegated to the bridge */ - if (dsi->conn.dev) - drm_connector_cleanup(&dsi->conn); + if (dsi->panel) + drm_panel_bridge_remove(dsi->bridge); }
static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp) @@ -881,20 +783,12 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host, dsi->format = device->format; dsi->mode_flags = device->mode_flags;
- if (dsi->conn.dev) - drm_helper_hpd_irq_event(dsi->conn.dev); - return 0; }
static int mtk_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) { - struct mtk_dsi *dsi = host_to_dsi(host); - - if (dsi->conn.dev) - drm_helper_hpd_irq_event(dsi->conn.dev); - return 0; }
@@ -1062,7 +956,7 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) goto err_ddp_comp_unregister; }
- ret = mtk_dsi_create_conn_enc(drm, dsi); + ret = mtk_dsi_create_enc(drm, dsi); if (ret) { DRM_ERROR("Encoder create failed with %d\n", ret); goto err_unregister; @@ -1114,6 +1008,13 @@ static int mtk_dsi_probe(struct platform_device *pdev) if (ret) return ret;
+ if (dsi->panel) { + dsi->bridge = drm_panel_bridge_add(dsi->panel, + DRM_MODE_CONNECTOR_DSI); + if (IS_ERR(dsi->bridge)) + return PTR_ERR(dsi->bridge); + } + dsi->engine_clk = devm_clk_get(dev, "engine"); if (IS_ERR(dsi->engine_clk)) { ret = PTR_ERR(dsi->engine_clk);
On Thu, 11 May 2017 11:31:25 -0700 Eric Anholt eric@anholt.net wrote:
Avoids a bunch of connector boilerplate. Note that this causes panel prepare() to be moved before mtk_dsi_poweron() and unprepare() to be after poweroff(). I think this is the expected usage of the panel API (enable should be when you do things that require the link to be brought up), but there may be issues here.
Signed-off-by: Eric Anholt eric@anholt.net
Reviewed-by: Boris Brezillon boris.brezillon@free-electrons.com
Note that I haven't tested this change, and am not committed to this patch. It's just an optional cleanup, if it works for you.
drivers/gpu/drm/mediatek/mtk_dsi.c | 125 ++++--------------------------------- 1 file changed, 13 insertions(+), 112 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 808b995a990f..764bd8b9c256 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -160,7 +160,6 @@ struct mtk_dsi { struct device *dev; struct mipi_dsi_host host; struct drm_encoder encoder;
- struct drm_connector conn; struct drm_panel *panel; struct drm_bridge *bridge; struct phy *phy;
@@ -188,11 +187,6 @@ static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e) return container_of(e, struct mtk_dsi, encoder); }
-static inline struct mtk_dsi *connector_to_dsi(struct drm_connector *c) -{
- return container_of(c, struct mtk_dsi, conn);
-}
static inline struct mtk_dsi *host_to_dsi(struct mipi_dsi_host *h) { return container_of(h, struct mtk_dsi, host); @@ -603,16 +597,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) mtk_dsi_lane0_ulp_mode_leave(dsi); mtk_dsi_clk_hs_mode(dsi, 0);
- if (dsi->panel) {
if (drm_panel_prepare(dsi->panel)) {DRM_ERROR("failed to prepare the panel\n");goto err_disable_digital_clk;}- }
- return 0;
-err_disable_digital_clk:
- clk_disable_unprepare(dsi->digital_clk);
err_disable_engine_clk: clk_disable_unprepare(dsi->engine_clk); err_phy_power_off: @@ -630,15 +615,7 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi) if (--dsi->refcount != 0) return;
- if (!mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500)) {
if (dsi->panel) {if (drm_panel_unprepare(dsi->panel)) {DRM_ERROR("failed to unprepare the panel\n");return;}}- }
- mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500); mtk_dsi_reset_engine(dsi); mtk_dsi_lane0_ulp_mode_enter(dsi); mtk_dsi_clk_ulp_mode_enter(dsi);
@@ -669,19 +646,9 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
mtk_dsi_start(dsi);
if (dsi->panel) {
if (drm_panel_enable(dsi->panel)) {DRM_ERROR("failed to enable the panel\n");goto err_dsi_power_off;}}
dsi->enabled = true;
return;
-err_dsi_power_off:
- mtk_dsi_stop(dsi);
- mtk_dsi_poweroff(dsi);
}
static void mtk_output_dsi_disable(struct mtk_dsi *dsi) @@ -689,13 +656,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) if (!dsi->enabled) return;
- if (dsi->panel) {
if (drm_panel_disable(dsi->panel)) {DRM_ERROR("failed to disable the panel\n");return;}- }
- mtk_dsi_stop(dsi); mtk_dsi_poweroff(dsi);
@@ -750,13 +710,6 @@ static void mtk_dsi_encoder_enable(struct drm_encoder *encoder) mtk_output_dsi_enable(dsi); }
-static int mtk_dsi_connector_get_modes(struct drm_connector *connector) -{
- struct mtk_dsi *dsi = connector_to_dsi(connector);
- return drm_panel_get_modes(dsi->panel);
-}
static const struct drm_encoder_helper_funcs mtk_dsi_encoder_helper_funcs = { .mode_fixup = mtk_dsi_encoder_mode_fixup, .mode_set = mtk_dsi_encoder_mode_set, @@ -764,52 +717,7 @@ static const struct drm_encoder_helper_funcs mtk_dsi_encoder_helper_funcs = { .enable = mtk_dsi_encoder_enable, };
-static const struct drm_connector_funcs mtk_dsi_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-static const struct drm_connector_helper_funcs
- mtk_dsi_connector_helper_funcs = {
- .get_modes = mtk_dsi_connector_get_modes,
-};
-static int mtk_dsi_create_connector(struct drm_device *drm, struct mtk_dsi *dsi) -{
- int ret;
- ret = drm_connector_init(drm, &dsi->conn, &mtk_dsi_connector_funcs,
DRM_MODE_CONNECTOR_DSI);- if (ret) {
DRM_ERROR("Failed to connector init to drm\n");return ret;- }
- drm_connector_helper_add(&dsi->conn, &mtk_dsi_connector_helper_funcs);
- dsi->conn.dpms = DRM_MODE_DPMS_OFF;
- drm_mode_connector_attach_encoder(&dsi->conn, &dsi->encoder);
- if (dsi->panel) {
ret = drm_panel_attach(dsi->panel, &dsi->conn);if (ret) {DRM_ERROR("Failed to attach panel to drm\n");goto err_connector_cleanup;}- }
- return 0;
-err_connector_cleanup:
- drm_connector_cleanup(&dsi->conn);
- return ret;
-}
-static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) +static int mtk_dsi_create_enc(struct drm_device *drm, struct mtk_dsi *dsi) { int ret;
@@ -827,15 +735,10 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) */ dsi->encoder.possible_crtcs = 1;
- /* If there's a bridge, attach to it and let it create the connector */ ret = drm_bridge_attach(&dsi->encoder, dsi->bridge, NULL); if (ret) { DRM_ERROR("Failed to attach bridge to drm\n");
/* Otherwise create our own connector and attach to a panel */ret = mtk_dsi_create_connector(drm, dsi);if (ret)goto err_encoder_cleanup;
goto err_encoder_cleanup;}
return 0;
@@ -848,9 +751,8 @@ static int mtk_dsi_create_conn_enc(struct drm_device *drm, struct mtk_dsi *dsi) static void mtk_dsi_destroy_conn_enc(struct mtk_dsi *dsi) { drm_encoder_cleanup(&dsi->encoder);
- /* Skip connector cleanup if creation was delegated to the bridge */
- if (dsi->conn.dev)
drm_connector_cleanup(&dsi->conn);
- if (dsi->panel)
drm_panel_bridge_remove(dsi->bridge);}
static void mtk_dsi_ddp_start(struct mtk_ddp_comp *comp) @@ -881,20 +783,12 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host, dsi->format = device->format; dsi->mode_flags = device->mode_flags;
- if (dsi->conn.dev)
drm_helper_hpd_irq_event(dsi->conn.dev);- return 0;
}
static int mtk_dsi_host_detach(struct mipi_dsi_host *host, struct mipi_dsi_device *device) {
- struct mtk_dsi *dsi = host_to_dsi(host);
- if (dsi->conn.dev)
drm_helper_hpd_irq_event(dsi->conn.dev);- return 0;
}
@@ -1062,7 +956,7 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) goto err_ddp_comp_unregister; }
- ret = mtk_dsi_create_conn_enc(drm, dsi);
- ret = mtk_dsi_create_enc(drm, dsi); if (ret) { DRM_ERROR("Encoder create failed with %d\n", ret); goto err_unregister;
@@ -1114,6 +1008,13 @@ static int mtk_dsi_probe(struct platform_device *pdev) if (ret) return ret;
- if (dsi->panel) {
dsi->bridge = drm_panel_bridge_add(dsi->panel,DRM_MODE_CONNECTOR_DSI);if (IS_ERR(dsi->bridge))return PTR_ERR(dsi->bridge);- }
- dsi->engine_clk = devm_clk_get(dev, "engine"); if (IS_ERR(dsi->engine_clk)) { ret = PTR_ERR(dsi->engine_clk);
Another 100 lines of boilerplate gone. Bridges aren't supported yet, but will be trivial to add later.
Signed-off-by: Eric Anholt eric@anholt.net ---
Also untested.
drivers/gpu/drm/stm/ltdc.c | 128 +++++---------------------------------------- drivers/gpu/drm/stm/ltdc.h | 2 +- 2 files changed, 13 insertions(+), 117 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index a40418cda74a..41a1c5d68f5b 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -269,11 +269,6 @@ static inline struct ltdc_device *encoder_to_ltdc(struct drm_encoder *enc) return (struct ltdc_device *)enc->dev->dev_private; }
-static inline struct ltdc_device *connector_to_ltdc(struct drm_connector *con) -{ - return (struct ltdc_device *)con->dev->dev_private; -} - static inline enum ltdc_pix_fmt to_ltdc_pixelformat(u32 drm_fmt) { enum ltdc_pix_fmt pf; @@ -815,22 +810,12 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
static void ltdc_rgb_encoder_enable(struct drm_encoder *encoder) { - struct ltdc_device *ldev = encoder_to_ltdc(encoder); - DRM_DEBUG_DRIVER("\n"); - - drm_panel_prepare(ldev->panel); - drm_panel_enable(ldev->panel); }
static void ltdc_rgb_encoder_disable(struct drm_encoder *encoder) { - struct ltdc_device *ldev = encoder_to_ltdc(encoder); - DRM_DEBUG_DRIVER("\n"); - - drm_panel_disable(ldev->panel); - drm_panel_unprepare(ldev->panel); }
static const struct drm_encoder_helper_funcs ltdc_rgb_encoder_helper_funcs = { @@ -863,82 +848,6 @@ static struct drm_encoder *ltdc_rgb_encoder_create(struct drm_device *ddev) return encoder; }
-/* - * DRM_CONNECTOR - */ - -static int ltdc_rgb_connector_get_modes(struct drm_connector *connector) -{ - struct drm_device *ddev = connector->dev; - struct ltdc_device *ldev = ddev->dev_private; - int ret = 0; - - DRM_DEBUG_DRIVER("\n"); - - if (ldev->panel) - ret = drm_panel_get_modes(ldev->panel); - - return ret < 0 ? 0 : ret; -} - -static struct drm_connector_helper_funcs ltdc_rgb_connector_helper_funcs = { - .get_modes = ltdc_rgb_connector_get_modes, -}; - -static enum drm_connector_status -ltdc_rgb_connector_detect(struct drm_connector *connector, bool force) -{ - struct ltdc_device *ldev = connector_to_ltdc(connector); - - return ldev->panel ? connector_status_connected : - connector_status_disconnected; -} - -static void ltdc_rgb_connector_destroy(struct drm_connector *connector) -{ - DRM_DEBUG_DRIVER("\n"); - - drm_connector_unregister(connector); - drm_connector_cleanup(connector); -} - -static const struct drm_connector_funcs ltdc_rgb_connector_funcs = { - .dpms = drm_atomic_helper_connector_dpms, - .fill_modes = drm_helper_probe_single_connector_modes, - .detect = ltdc_rgb_connector_detect, - .destroy = ltdc_rgb_connector_destroy, - .reset = drm_atomic_helper_connector_reset, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - -struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev) -{ - struct drm_connector *connector; - int err; - - connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL); - if (!connector) { - DRM_ERROR("Failed to allocate connector\n"); - return NULL; - } - - connector->polled = DRM_CONNECTOR_POLL_HPD; - - err = drm_connector_init(ddev, connector, <dc_rgb_connector_funcs, - DRM_MODE_CONNECTOR_DPI); - if (err) { - DRM_ERROR("Failed to initialize connector\n"); - return NULL; - } - - drm_connector_helper_add(connector, <dc_rgb_connector_helper_funcs); - - DRM_DEBUG_DRIVER("RGB connector:%d created\n", connector->base.id); - - return connector; -} - static int ltdc_get_caps(struct drm_device *ddev) { struct ltdc_device *ldev = ddev->dev_private; @@ -972,7 +881,7 @@ static int ltdc_get_caps(struct drm_device *ddev) return 0; }
-static struct drm_panel *ltdc_get_panel(struct drm_device *ddev) +static struct drm_bridge *ltdc_get_bridge(struct drm_device *ddev) { struct device *dev = ddev->dev; struct device_node *np = dev->of_node; @@ -1004,7 +913,10 @@ static struct drm_panel *ltdc_get_panel(struct drm_device *ddev) } }
- return panel; + if (!panel) + return ERR_PTR(-ENODEV); + + return drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DPI); }
int ltdc_load(struct drm_device *ddev) @@ -1014,7 +926,6 @@ int ltdc_load(struct drm_device *ddev) struct device *dev = ddev->dev; struct device_node *np = dev->of_node; struct drm_encoder *encoder; - struct drm_connector *connector = NULL; struct drm_crtc *crtc; struct reset_control *rstc; struct resource res; @@ -1022,8 +933,8 @@ int ltdc_load(struct drm_device *ddev)
DRM_DEBUG_DRIVER("\n");
- ldev->panel = ltdc_get_panel(ddev); - if (!ldev->panel) + ldev->bridge = ltdc_get_bridge(ddev); + if (!ldev->bridge) return -EPROBE_DEFER;
rstc = of_reset_control_get(np, NULL); @@ -1082,28 +993,13 @@ int ltdc_load(struct drm_device *ddev)
DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
- if (ldev->panel) { + if (ldev->bridge) { encoder = ltdc_rgb_encoder_create(ddev); if (!encoder) { DRM_ERROR("Failed to create RGB encoder\n"); ret = -EINVAL; goto err; } - - connector = ltdc_rgb_connector_create(ddev); - if (!connector) { - DRM_ERROR("Failed to create RGB connector\n"); - ret = -EINVAL; - goto err; - } - - ret = drm_mode_connector_attach_encoder(connector, encoder); - if (ret) { - DRM_ERROR("Failed to attach connector to encoder\n"); - goto err; - } - - drm_panel_attach(ldev->panel, connector); }
crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL); @@ -1130,8 +1026,8 @@ int ltdc_load(struct drm_device *ddev)
return 0; err: - if (ldev->panel) - drm_panel_detach(ldev->panel); + if (ldev->bridge) + drm_panel_bridge_remove(ldev->bridge);
clk_disable_unprepare(ldev->pixel_clk);
@@ -1146,8 +1042,8 @@ void ltdc_unload(struct drm_device *ddev)
drm_vblank_cleanup(ddev);
- if (ldev->panel) - drm_panel_detach(ldev->panel); + if (ldev->bridge) + drm_panel_bridge_remove(ldev->bridge);
clk_disable_unprepare(ldev->pixel_clk); } diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h index d7a9c736ac1e..d78cb0dd3200 100644 --- a/drivers/gpu/drm/stm/ltdc.h +++ b/drivers/gpu/drm/stm/ltdc.h @@ -24,7 +24,7 @@ struct ltdc_device { struct drm_fbdev_cma *fbdev; void __iomem *regs; struct clk *pixel_clk; /* lcd pixel clock */ - struct drm_panel *panel; + struct drm_bridge *bridge; struct mutex err_lock; /* protecting error_status */ struct ltdc_caps caps; u32 clut[256]; /* color look up table */
On Thu, 11 May 2017 11:31:26 -0700 Eric Anholt eric@anholt.net wrote:
Another 100 lines of boilerplate gone. Bridges aren't supported yet, but will be trivial to add later.
Signed-off-by: Eric Anholt eric@anholt.net
[...]
@@ -1082,28 +993,13 @@ int ltdc_load(struct drm_device *ddev)
DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
- if (ldev->panel) {
- if (ldev->bridge) { encoder = ltdc_rgb_encoder_create(ddev); if (!encoder) { DRM_ERROR("Failed to create RGB encoder\n"); ret = -EINVAL; goto err; }
You should have a call to drm_bridge_attach() somewhere, otherwise your panel (and its connector) will not be registered.
connector = ltdc_rgb_connector_create(ddev);if (!connector) {DRM_ERROR("Failed to create RGB connector\n");ret = -EINVAL;goto err;}ret = drm_mode_connector_attach_encoder(connector, encoder);if (ret) {DRM_ERROR("Failed to attach connector to encoder\n");goto err;}drm_panel_attach(ldev->panel, connector);
Hi Eric,
I took your patch for the panel-bridge and it works perfectly in both DPI mode (panel RGB //) and DSI mode (bridge dw mipi dsi), bravo :-)
~160 lines have been removed from ltdc.c thanks to your panel-bridge code!
Many thanks Philippe
On 05/11/2017 08:31 PM, Eric Anholt wrote:
Another 100 lines of boilerplate gone. Bridges aren't supported yet, but will be trivial to add later.
Signed-off-by: Eric Anholt eric@anholt.net
Also untested.
drivers/gpu/drm/stm/ltdc.c | 128 +++++---------------------------------------- drivers/gpu/drm/stm/ltdc.h | 2 +- 2 files changed, 13 insertions(+), 117 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index a40418cda74a..41a1c5d68f5b 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -269,11 +269,6 @@ static inline struct ltdc_device *encoder_to_ltdc(struct drm_encoder *enc) return (struct ltdc_device *)enc->dev->dev_private; }
-static inline struct ltdc_device *connector_to_ltdc(struct drm_connector *con) -{
- return (struct ltdc_device *)con->dev->dev_private;
-}
- static inline enum ltdc_pix_fmt to_ltdc_pixelformat(u32 drm_fmt) { enum ltdc_pix_fmt pf;
@@ -815,22 +810,12 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
static void ltdc_rgb_encoder_enable(struct drm_encoder *encoder) {
struct ltdc_device *ldev = encoder_to_ltdc(encoder);
DRM_DEBUG_DRIVER("\n");
drm_panel_prepare(ldev->panel);
drm_panel_enable(ldev->panel); }
static void ltdc_rgb_encoder_disable(struct drm_encoder *encoder) {
struct ltdc_device *ldev = encoder_to_ltdc(encoder);
DRM_DEBUG_DRIVER("\n");
drm_panel_disable(ldev->panel);
drm_panel_unprepare(ldev->panel); }
static const struct drm_encoder_helper_funcs ltdc_rgb_encoder_helper_funcs = {
@@ -863,82 +848,6 @@ static struct drm_encoder *ltdc_rgb_encoder_create(struct drm_device *ddev) return encoder; }
-/*
- DRM_CONNECTOR
- */
-static int ltdc_rgb_connector_get_modes(struct drm_connector *connector) -{
- struct drm_device *ddev = connector->dev;
- struct ltdc_device *ldev = ddev->dev_private;
- int ret = 0;
- DRM_DEBUG_DRIVER("\n");
- if (ldev->panel)
ret = drm_panel_get_modes(ldev->panel);- return ret < 0 ? 0 : ret;
-}
-static struct drm_connector_helper_funcs ltdc_rgb_connector_helper_funcs = {
- .get_modes = ltdc_rgb_connector_get_modes,
-};
-static enum drm_connector_status -ltdc_rgb_connector_detect(struct drm_connector *connector, bool force) -{
- struct ltdc_device *ldev = connector_to_ltdc(connector);
- return ldev->panel ? connector_status_connected :
connector_status_disconnected;-}
-static void ltdc_rgb_connector_destroy(struct drm_connector *connector) -{
- DRM_DEBUG_DRIVER("\n");
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-static const struct drm_connector_funcs ltdc_rgb_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .detect = ltdc_rgb_connector_detect,
- .destroy = ltdc_rgb_connector_destroy,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev) -{
- struct drm_connector *connector;
- int err;
- connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL);
- if (!connector) {
DRM_ERROR("Failed to allocate connector\n");return NULL;- }
- connector->polled = DRM_CONNECTOR_POLL_HPD;
- err = drm_connector_init(ddev, connector, <dc_rgb_connector_funcs,
DRM_MODE_CONNECTOR_DPI);- if (err) {
DRM_ERROR("Failed to initialize connector\n");return NULL;- }
- drm_connector_helper_add(connector, <dc_rgb_connector_helper_funcs);
- DRM_DEBUG_DRIVER("RGB connector:%d created\n", connector->base.id);
- return connector;
-}
- static int ltdc_get_caps(struct drm_device *ddev) { struct ltdc_device *ldev = ddev->dev_private;
@@ -972,7 +881,7 @@ static int ltdc_get_caps(struct drm_device *ddev) return 0; }
-static struct drm_panel *ltdc_get_panel(struct drm_device *ddev) +static struct drm_bridge *ltdc_get_bridge(struct drm_device *ddev) { struct device *dev = ddev->dev; struct device_node *np = dev->of_node; @@ -1004,7 +913,10 @@ static struct drm_panel *ltdc_get_panel(struct drm_device *ddev) } }
- return panel;
if (!panel)
return ERR_PTR(-ENODEV);return drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DPI); }
int ltdc_load(struct drm_device *ddev)
@@ -1014,7 +926,6 @@ int ltdc_load(struct drm_device *ddev) struct device *dev = ddev->dev; struct device_node *np = dev->of_node; struct drm_encoder *encoder;
- struct drm_connector *connector = NULL; struct drm_crtc *crtc; struct reset_control *rstc; struct resource res;
@@ -1022,8 +933,8 @@ int ltdc_load(struct drm_device *ddev)
DRM_DEBUG_DRIVER("\n");
- ldev->panel = ltdc_get_panel(ddev);
- if (!ldev->panel)
ldev->bridge = ltdc_get_bridge(ddev);
if (!ldev->bridge) return -EPROBE_DEFER;
rstc = of_reset_control_get(np, NULL);
@@ -1082,28 +993,13 @@ int ltdc_load(struct drm_device *ddev)
DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
- if (ldev->panel) {
- if (ldev->bridge) { encoder = ltdc_rgb_encoder_create(ddev); if (!encoder) { DRM_ERROR("Failed to create RGB encoder\n"); ret = -EINVAL; goto err; }
connector = ltdc_rgb_connector_create(ddev);if (!connector) {DRM_ERROR("Failed to create RGB connector\n");ret = -EINVAL;goto err;}ret = drm_mode_connector_attach_encoder(connector, encoder);if (ret) {DRM_ERROR("Failed to attach connector to encoder\n");goto err;}drm_panel_attach(ldev->panel, connector);}
crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL);
@@ -1130,8 +1026,8 @@ int ltdc_load(struct drm_device *ddev)
return 0; err:
- if (ldev->panel)
drm_panel_detach(ldev->panel);
if (ldev->bridge)
drm_panel_bridge_remove(ldev->bridge);clk_disable_unprepare(ldev->pixel_clk);
@@ -1146,8 +1042,8 @@ void ltdc_unload(struct drm_device *ddev)
drm_vblank_cleanup(ddev);
- if (ldev->panel)
drm_panel_detach(ldev->panel);
if (ldev->bridge)
drm_panel_bridge_remove(ldev->bridge);clk_disable_unprepare(ldev->pixel_clk); }
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h index d7a9c736ac1e..d78cb0dd3200 100644 --- a/drivers/gpu/drm/stm/ltdc.h +++ b/drivers/gpu/drm/stm/ltdc.h @@ -24,7 +24,7 @@ struct ltdc_device { struct drm_fbdev_cma *fbdev; void __iomem *regs; struct clk *pixel_clk; /* lcd pixel clock */
- struct drm_panel *panel;
- struct drm_bridge *bridge; struct mutex err_lock; /* protecting error_status */ struct ltdc_caps caps; u32 clut[256]; /* color look up table */
Hi Philippe,
Le Tue, 30 May 2017 16:55:42 +0000, Philippe CORNU philippe.cornu@st.com a écrit :
Hi Eric,
I took your patch for the panel-bridge and it works perfectly in both DPI mode (panel RGB //) and DSI mode (bridge dw mipi dsi), bravo :-)
I still don't understand how it can work without a call to drm_bridge_attach() (which is used to link the RGB encoder to the DPI connector). I'm probably missing something obvious. Maybe someone can point it out :-).
Thanks,
Boris
~160 lines have been removed from ltdc.c thanks to your panel-bridge code!
Many thanks Philippe
On 05/11/2017 08:31 PM, Eric Anholt wrote:
Another 100 lines of boilerplate gone. Bridges aren't supported yet, but will be trivial to add later.
Signed-off-by: Eric Anholt eric@anholt.net
Also untested.
drivers/gpu/drm/stm/ltdc.c | 128 +++++---------------------------------------- drivers/gpu/drm/stm/ltdc.h | 2 +- 2 files changed, 13 insertions(+), 117 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index a40418cda74a..41a1c5d68f5b 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -269,11 +269,6 @@ static inline struct ltdc_device *encoder_to_ltdc(struct drm_encoder *enc) return (struct ltdc_device *)enc->dev->dev_private; }
-static inline struct ltdc_device *connector_to_ltdc(struct drm_connector *con) -{
- return (struct ltdc_device *)con->dev->dev_private;
-}
- static inline enum ltdc_pix_fmt to_ltdc_pixelformat(u32 drm_fmt) { enum ltdc_pix_fmt pf;
@@ -815,22 +810,12 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
static void ltdc_rgb_encoder_enable(struct drm_encoder *encoder) {
struct ltdc_device *ldev = encoder_to_ltdc(encoder);
DRM_DEBUG_DRIVER("\n");
drm_panel_prepare(ldev->panel);
drm_panel_enable(ldev->panel); }
static void ltdc_rgb_encoder_disable(struct drm_encoder *encoder) {
struct ltdc_device *ldev = encoder_to_ltdc(encoder);
DRM_DEBUG_DRIVER("\n");
drm_panel_disable(ldev->panel);
drm_panel_unprepare(ldev->panel); }
static const struct drm_encoder_helper_funcs ltdc_rgb_encoder_helper_funcs = {
@@ -863,82 +848,6 @@ static struct drm_encoder *ltdc_rgb_encoder_create(struct drm_device *ddev) return encoder; }
-/*
- DRM_CONNECTOR
- */
-static int ltdc_rgb_connector_get_modes(struct drm_connector *connector) -{
- struct drm_device *ddev = connector->dev;
- struct ltdc_device *ldev = ddev->dev_private;
- int ret = 0;
- DRM_DEBUG_DRIVER("\n");
- if (ldev->panel)
ret = drm_panel_get_modes(ldev->panel);- return ret < 0 ? 0 : ret;
-}
-static struct drm_connector_helper_funcs ltdc_rgb_connector_helper_funcs = {
- .get_modes = ltdc_rgb_connector_get_modes,
-};
-static enum drm_connector_status -ltdc_rgb_connector_detect(struct drm_connector *connector, bool force) -{
- struct ltdc_device *ldev = connector_to_ltdc(connector);
- return ldev->panel ? connector_status_connected :
connector_status_disconnected;-}
-static void ltdc_rgb_connector_destroy(struct drm_connector *connector) -{
- DRM_DEBUG_DRIVER("\n");
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-static const struct drm_connector_funcs ltdc_rgb_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .detect = ltdc_rgb_connector_detect,
- .destroy = ltdc_rgb_connector_destroy,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev) -{
- struct drm_connector *connector;
- int err;
- connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL);
- if (!connector) {
DRM_ERROR("Failed to allocate connector\n");return NULL;- }
- connector->polled = DRM_CONNECTOR_POLL_HPD;
- err = drm_connector_init(ddev, connector, <dc_rgb_connector_funcs,
DRM_MODE_CONNECTOR_DPI);- if (err) {
DRM_ERROR("Failed to initialize connector\n");return NULL;- }
- drm_connector_helper_add(connector, <dc_rgb_connector_helper_funcs);
- DRM_DEBUG_DRIVER("RGB connector:%d created\n", connector->base.id);
- return connector;
-}
- static int ltdc_get_caps(struct drm_device *ddev) { struct ltdc_device *ldev = ddev->dev_private;
@@ -972,7 +881,7 @@ static int ltdc_get_caps(struct drm_device *ddev) return 0; }
-static struct drm_panel *ltdc_get_panel(struct drm_device *ddev) +static struct drm_bridge *ltdc_get_bridge(struct drm_device *ddev) { struct device *dev = ddev->dev; struct device_node *np = dev->of_node; @@ -1004,7 +913,10 @@ static struct drm_panel *ltdc_get_panel(struct drm_device *ddev) } }
- return panel;
if (!panel)
return ERR_PTR(-ENODEV);return drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DPI); }
int ltdc_load(struct drm_device *ddev)
@@ -1014,7 +926,6 @@ int ltdc_load(struct drm_device *ddev) struct device *dev = ddev->dev; struct device_node *np = dev->of_node; struct drm_encoder *encoder;
- struct drm_connector *connector = NULL; struct drm_crtc *crtc; struct reset_control *rstc; struct resource res;
@@ -1022,8 +933,8 @@ int ltdc_load(struct drm_device *ddev)
DRM_DEBUG_DRIVER("\n");
- ldev->panel = ltdc_get_panel(ddev);
- if (!ldev->panel)
ldev->bridge = ltdc_get_bridge(ddev);
if (!ldev->bridge) return -EPROBE_DEFER;
rstc = of_reset_control_get(np, NULL);
@@ -1082,28 +993,13 @@ int ltdc_load(struct drm_device *ddev)
DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
- if (ldev->panel) {
- if (ldev->bridge) { encoder = ltdc_rgb_encoder_create(ddev); if (!encoder) { DRM_ERROR("Failed to create RGB encoder\n"); ret = -EINVAL; goto err; }
connector = ltdc_rgb_connector_create(ddev);if (!connector) {DRM_ERROR("Failed to create RGB connector\n");ret = -EINVAL;goto err;}ret = drm_mode_connector_attach_encoder(connector, encoder);if (ret) {DRM_ERROR("Failed to attach connector to encoder\n");goto err;}drm_panel_attach(ldev->panel, connector);}
crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL);
@@ -1130,8 +1026,8 @@ int ltdc_load(struct drm_device *ddev)
return 0; err:
- if (ldev->panel)
drm_panel_detach(ldev->panel);
if (ldev->bridge)
drm_panel_bridge_remove(ldev->bridge);clk_disable_unprepare(ldev->pixel_clk);
@@ -1146,8 +1042,8 @@ void ltdc_unload(struct drm_device *ddev)
drm_vblank_cleanup(ddev);
- if (ldev->panel)
drm_panel_detach(ldev->panel);
if (ldev->bridge)
drm_panel_bridge_remove(ldev->bridge);clk_disable_unprepare(ldev->pixel_clk); }
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h index d7a9c736ac1e..d78cb0dd3200 100644 --- a/drivers/gpu/drm/stm/ltdc.h +++ b/drivers/gpu/drm/stm/ltdc.h @@ -24,7 +24,7 @@ struct ltdc_device { struct drm_fbdev_cma *fbdev; void __iomem *regs; struct clk *pixel_clk; /* lcd pixel clock */
- struct drm_panel *panel;
- struct drm_bridge *bridge; struct mutex err_lock; /* protecting error_status */ struct ltdc_caps caps; u32 clut[256]; /* color look up table */
Hi Boris,
On 05/31/2017 11:56 AM, Boris Brezillon wrote:
Hi Philippe,
Le Tue, 30 May 2017 16:55:42 +0000, Philippe CORNU philippe.cornu@st.com a écrit :
Hi Eric,
I took your patch for the panel-bridge and it works perfectly in both DPI mode (panel RGB //) and DSI mode (bridge dw mipi dsi), bravo :-)
I still don't understand how it can work without a call to drm_bridge_attach() (which is used to link the RGB encoder to the DPI connector). I'm probably missing something obvious. Maybe someone can point it out :-).
I think the expectation is that there will be a follow-up patch that would add bridge support. Philippe had posted a patch for adding bridges to ltdc before:
https://patchwork.kernel.org/patch/9724835/
He can now redo his patch over Eric's panel-bridge patch-set.
Thanks, Archit
Thanks,
Boris
~160 lines have been removed from ltdc.c thanks to your panel-bridge code!
Many thanks Philippe
On 05/11/2017 08:31 PM, Eric Anholt wrote:
Another 100 lines of boilerplate gone. Bridges aren't supported yet, but will be trivial to add later.
Signed-off-by: Eric Anholt eric@anholt.net
Also untested.
drivers/gpu/drm/stm/ltdc.c | 128 +++++---------------------------------------- drivers/gpu/drm/stm/ltdc.h | 2 +- 2 files changed, 13 insertions(+), 117 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index a40418cda74a..41a1c5d68f5b 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -269,11 +269,6 @@ static inline struct ltdc_device *encoder_to_ltdc(struct drm_encoder *enc) return (struct ltdc_device *)enc->dev->dev_private; }
-static inline struct ltdc_device *connector_to_ltdc(struct drm_connector *con) -{
- return (struct ltdc_device *)con->dev->dev_private;
-}
- static inline enum ltdc_pix_fmt to_ltdc_pixelformat(u32 drm_fmt) { enum ltdc_pix_fmt pf;
@@ -815,22 +810,12 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
static void ltdc_rgb_encoder_enable(struct drm_encoder *encoder) {
struct ltdc_device *ldev = encoder_to_ltdc(encoder);
DRM_DEBUG_DRIVER("\n");
drm_panel_prepare(ldev->panel);
drm_panel_enable(ldev->panel); }
static void ltdc_rgb_encoder_disable(struct drm_encoder *encoder) {
struct ltdc_device *ldev = encoder_to_ltdc(encoder);
DRM_DEBUG_DRIVER("\n");
drm_panel_disable(ldev->panel);
drm_panel_unprepare(ldev->panel); }
static const struct drm_encoder_helper_funcs ltdc_rgb_encoder_helper_funcs = {
@@ -863,82 +848,6 @@ static struct drm_encoder *ltdc_rgb_encoder_create(struct drm_device *ddev) return encoder; }
-/*
- DRM_CONNECTOR
- */
-static int ltdc_rgb_connector_get_modes(struct drm_connector *connector) -{
- struct drm_device *ddev = connector->dev;
- struct ltdc_device *ldev = ddev->dev_private;
- int ret = 0;
- DRM_DEBUG_DRIVER("\n");
- if (ldev->panel)
ret = drm_panel_get_modes(ldev->panel);- return ret < 0 ? 0 : ret;
-}
-static struct drm_connector_helper_funcs ltdc_rgb_connector_helper_funcs = {
- .get_modes = ltdc_rgb_connector_get_modes,
-};
-static enum drm_connector_status -ltdc_rgb_connector_detect(struct drm_connector *connector, bool force) -{
- struct ltdc_device *ldev = connector_to_ltdc(connector);
- return ldev->panel ? connector_status_connected :
connector_status_disconnected;-}
-static void ltdc_rgb_connector_destroy(struct drm_connector *connector) -{
- DRM_DEBUG_DRIVER("\n");
- drm_connector_unregister(connector);
- drm_connector_cleanup(connector);
-}
-static const struct drm_connector_funcs ltdc_rgb_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .detect = ltdc_rgb_connector_detect,
- .destroy = ltdc_rgb_connector_destroy,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev) -{
- struct drm_connector *connector;
- int err;
- connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL);
- if (!connector) {
DRM_ERROR("Failed to allocate connector\n");return NULL;- }
- connector->polled = DRM_CONNECTOR_POLL_HPD;
- err = drm_connector_init(ddev, connector, <dc_rgb_connector_funcs,
DRM_MODE_CONNECTOR_DPI);- if (err) {
DRM_ERROR("Failed to initialize connector\n");return NULL;- }
- drm_connector_helper_add(connector, <dc_rgb_connector_helper_funcs);
- DRM_DEBUG_DRIVER("RGB connector:%d created\n", connector->base.id);
- return connector;
-}
- static int ltdc_get_caps(struct drm_device *ddev) { struct ltdc_device *ldev = ddev->dev_private;
@@ -972,7 +881,7 @@ static int ltdc_get_caps(struct drm_device *ddev) return 0; }
-static struct drm_panel *ltdc_get_panel(struct drm_device *ddev) +static struct drm_bridge *ltdc_get_bridge(struct drm_device *ddev) { struct device *dev = ddev->dev; struct device_node *np = dev->of_node; @@ -1004,7 +913,10 @@ static struct drm_panel *ltdc_get_panel(struct drm_device *ddev) } }
- return panel;
if (!panel)
return ERR_PTR(-ENODEV);return drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DPI); }
int ltdc_load(struct drm_device *ddev)
@@ -1014,7 +926,6 @@ int ltdc_load(struct drm_device *ddev) struct device *dev = ddev->dev; struct device_node *np = dev->of_node; struct drm_encoder *encoder;
- struct drm_connector *connector = NULL; struct drm_crtc *crtc; struct reset_control *rstc; struct resource res;
@@ -1022,8 +933,8 @@ int ltdc_load(struct drm_device *ddev)
DRM_DEBUG_DRIVER("\n");
- ldev->panel = ltdc_get_panel(ddev);
- if (!ldev->panel)
ldev->bridge = ltdc_get_bridge(ddev);
if (!ldev->bridge) return -EPROBE_DEFER;
rstc = of_reset_control_get(np, NULL);
@@ -1082,28 +993,13 @@ int ltdc_load(struct drm_device *ddev)
DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
- if (ldev->panel) {
- if (ldev->bridge) { encoder = ltdc_rgb_encoder_create(ddev); if (!encoder) { DRM_ERROR("Failed to create RGB encoder\n"); ret = -EINVAL; goto err; }
connector = ltdc_rgb_connector_create(ddev);if (!connector) {DRM_ERROR("Failed to create RGB connector\n");ret = -EINVAL;goto err;}ret = drm_mode_connector_attach_encoder(connector, encoder);if (ret) {DRM_ERROR("Failed to attach connector to encoder\n");goto err;}drm_panel_attach(ldev->panel, connector);}
crtc = devm_kzalloc(dev, sizeof(*crtc), GFP_KERNEL);
@@ -1130,8 +1026,8 @@ int ltdc_load(struct drm_device *ddev)
return 0; err:
- if (ldev->panel)
drm_panel_detach(ldev->panel);
if (ldev->bridge)
drm_panel_bridge_remove(ldev->bridge);clk_disable_unprepare(ldev->pixel_clk);
@@ -1146,8 +1042,8 @@ void ltdc_unload(struct drm_device *ddev)
drm_vblank_cleanup(ddev);
- if (ldev->panel)
drm_panel_detach(ldev->panel);
if (ldev->bridge)
drm_panel_bridge_remove(ldev->bridge);clk_disable_unprepare(ldev->pixel_clk); }
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h index d7a9c736ac1e..d78cb0dd3200 100644 --- a/drivers/gpu/drm/stm/ltdc.h +++ b/drivers/gpu/drm/stm/ltdc.h @@ -24,7 +24,7 @@ struct ltdc_device { struct drm_fbdev_cma *fbdev; void __iomem *regs; struct clk *pixel_clk; /* lcd pixel clock */
- struct drm_panel *panel;
- struct drm_bridge *bridge; struct mutex err_lock; /* protecting error_status */ struct ltdc_caps caps; u32 clut[256]; /* color look up table */
Hi Archit,
Le Wed, 31 May 2017 13:31:16 +0530, Archit Taneja architt@codeaurora.org a écrit :
Hi Boris,
On 05/31/2017 11:56 AM, Boris Brezillon wrote:
Hi Philippe,
Le Tue, 30 May 2017 16:55:42 +0000, Philippe CORNU philippe.cornu@st.com a écrit :
Hi Eric,
I took your patch for the panel-bridge and it works perfectly in both DPI mode (panel RGB //) and DSI mode (bridge dw mipi dsi), bravo :-)
I still don't understand how it can work without a call to drm_bridge_attach() (which is used to link the RGB encoder to the DPI connector). I'm probably missing something obvious. Maybe someone can point it out :-).
I think the expectation is that there will be a follow-up patch that would add bridge support. Philippe had posted a patch for adding bridges to ltdc before:
https://patchwork.kernel.org/patch/9724835/
He can now redo his patch over Eric's panel-bridge patch-set.
But by doing that you're breaking bisectability, or is it working when applying only Eric's patchset?
If applying this patch breaks the driver, why not applying it after bridge support has been added to this driver?
Regards,
Boris
On 05/31/2017 01:37 PM, Boris Brezillon wrote:
Hi Archit,
Le Wed, 31 May 2017 13:31:16 +0530, Archit Taneja architt@codeaurora.org a écrit :
Hi Boris,
On 05/31/2017 11:56 AM, Boris Brezillon wrote:
Hi Philippe,
Le Tue, 30 May 2017 16:55:42 +0000, Philippe CORNU philippe.cornu@st.com a écrit :
Hi Eric,
I took your patch for the panel-bridge and it works perfectly in both DPI mode (panel RGB //) and DSI mode (bridge dw mipi dsi), bravo :-)
I still don't understand how it can work without a call to drm_bridge_attach() (which is used to link the RGB encoder to the DPI connector). I'm probably missing something obvious. Maybe someone can point it out :-).
I think the expectation is that there will be a follow-up patch that would add bridge support. Philippe had posted a patch for adding bridges to ltdc before:
https://patchwork.kernel.org/patch/9724835/
He can now redo his patch over Eric's panel-bridge patch-set.
But by doing that you're breaking bisectability, or is it working when applying only Eric's patchset?
If applying this patch breaks the driver, why not applying it after bridge support has been added to this driver?
Yeah, it would break the driver. Philippe can create a patch that adds bridge support using the panel-bridge glue layer using Eric's patch as a reference. In the end, there should only be a single patch.
Thanks, Archit
Regards,
Boris
On 05/31/2017 10:35 AM, Archit Taneja wrote:
On 05/31/2017 01:37 PM, Boris Brezillon wrote:
Hi Archit,
Le Wed, 31 May 2017 13:31:16 +0530, Archit Taneja architt@codeaurora.org a écrit :
Hi Boris,
On 05/31/2017 11:56 AM, Boris Brezillon wrote:
Hi Philippe,
Le Tue, 30 May 2017 16:55:42 +0000, Philippe CORNU philippe.cornu@st.com a écrit :
Hi Eric,
I took your patch for the panel-bridge and it works perfectly in both DPI mode (panel RGB //) and DSI mode (bridge dw mipi dsi), bravo :-)
I still don't understand how it can work without a call to drm_bridge_attach() (which is used to link the RGB encoder to the DPI connector). I'm probably missing something obvious. Maybe someone can point it out :-).
I think the expectation is that there will be a follow-up patch that would add bridge support. Philippe had posted a patch for adding bridges to ltdc before:
https://patchwork.kernel.org/patch/9724835/
He can now redo his patch over Eric's panel-bridge patch-set.
But by doing that you're breaking bisectability, or is it working when applying only Eric's patchset?
If applying this patch breaks the driver, why not applying it after bridge support has been added to this driver?
Yeah, it would break the driver. Philippe can create a patch that adds bridge support using the panel-bridge glue layer using Eric's patch as a reference. In the end, there should only be a single patch.
Thanks, Archit
Regards,
Boris
Hi Boris & Archit,
Sorry, I should have been clearer about what I did. Let me explain more below:
1 - I applied Eric's patch named "drm/bridge: Refactor out the panel wrapper from the lvds-encoder bridge" (https://lkml.org/lkml/2017/5/11/784)
2 - I did a patch on ltdc.c combining Eric's patch named "drm/ltdc: Use the panel-bridge helper" (https://lkml.org/lkml/2017/5/11/783) and my own preamble patch for STM32 DSI named "drm/stm: ltdc: Add bridge support" (https://patchwork.kernel.org/patch/9738427/)
And tests are successful in both DPI mode (panel RGB //) and DSI mode (bridge dw mipi dsi).
As suggested in the last Archit's email, there will be a single patch that will add bridge support to ltdc.c and I will send it in the "STM32 DSI HOST" v3 version, if you all agree of course :)
Many thanks for all your great comments, Philippe
drm_encoder_cleanup() finishes with memsetting it to 0, already.
Signed-off-by: Eric Anholt eric@anholt.net --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c index 65a3bd7a0c00..4b2cfbd0d43f 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c @@ -83,14 +83,8 @@ static const struct drm_encoder_helper_funcs atmel_hlcdc_panel_encoder_helper_fu .enable = atmel_hlcdc_rgb_encoder_enable, };
-static void atmel_hlcdc_rgb_encoder_destroy(struct drm_encoder *encoder) -{ - drm_encoder_cleanup(encoder); - memset(encoder, 0, sizeof(*encoder)); -} - static const struct drm_encoder_funcs atmel_hlcdc_panel_encoder_funcs = { - .destroy = atmel_hlcdc_rgb_encoder_destroy, + .destroy = drm_encoder_cleanup, };
static int atmel_hlcdc_panel_get_modes(struct drm_connector *connector)
On Thu, 11 May 2017 11:31:27 -0700 Eric Anholt eric@anholt.net wrote:
drm_encoder_cleanup() finishes with memsetting it to 0, already.
Signed-off-by: Eric Anholt eric@anholt.net
Acked-by: Boris Brezillon boris.brezillon@free-electrons.com
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c index 65a3bd7a0c00..4b2cfbd0d43f 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c @@ -83,14 +83,8 @@ static const struct drm_encoder_helper_funcs atmel_hlcdc_panel_encoder_helper_fu .enable = atmel_hlcdc_rgb_encoder_enable, };
-static void atmel_hlcdc_rgb_encoder_destroy(struct drm_encoder *encoder) -{
- drm_encoder_cleanup(encoder);
- memset(encoder, 0, sizeof(*encoder));
-}
static const struct drm_encoder_funcs atmel_hlcdc_panel_encoder_funcs = {
- .destroy = atmel_hlcdc_rgb_encoder_destroy,
- .destroy = drm_encoder_cleanup,
};
static int atmel_hlcdc_panel_get_modes(struct drm_connector *connector)
This cuts 135 lines of boilerplate, at the cost of losing the filtering of get_modes() using atmel_hlcdc_dc_mode_valid(). The atomic check will still check that we don't set an invalid mode, though.
Signed-off-by: Eric Anholt eric@anholt.net ---
This patch is just a proposal for atmel-hlcdc if you like it -- maybe you still want the filtering of get_modes(). I'm not sure from a userspace API perspective if we should really have the encoder (your CRTC's limits) filtering modes from a connector, but I'm also pretty sure it doesn't actually matter.
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 163 ++--------------------- 1 file changed, 14 insertions(+), 149 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c index 4b2cfbd0d43f..340ef962aa81 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c @@ -23,191 +23,56 @@
#include <drm/drmP.h> #include <drm/drm_of.h> +#include <drm/drm_bridge.h>
#include "atmel_hlcdc_dc.h"
-/** - * Atmel HLCDC RGB connector structure - * - * This structure stores RGB slave device information. - * - * @connector: DRM connector - * @encoder: DRM encoder - * @dc: pointer to the atmel_hlcdc_dc structure - * @panel: panel connected on the RGB output - */ -struct atmel_hlcdc_rgb_output { - struct drm_connector connector; - struct drm_encoder encoder; - struct atmel_hlcdc_dc *dc; - struct drm_panel *panel; -}; - -static inline struct atmel_hlcdc_rgb_output * -drm_connector_to_atmel_hlcdc_rgb_output(struct drm_connector *connector) -{ - return container_of(connector, struct atmel_hlcdc_rgb_output, - connector); -} - -static inline struct atmel_hlcdc_rgb_output * -drm_encoder_to_atmel_hlcdc_rgb_output(struct drm_encoder *encoder) -{ - return container_of(encoder, struct atmel_hlcdc_rgb_output, encoder); -} - -static void atmel_hlcdc_rgb_encoder_enable(struct drm_encoder *encoder) -{ - struct atmel_hlcdc_rgb_output *rgb = - drm_encoder_to_atmel_hlcdc_rgb_output(encoder); - - if (rgb->panel) { - drm_panel_prepare(rgb->panel); - drm_panel_enable(rgb->panel); - } -} - -static void atmel_hlcdc_rgb_encoder_disable(struct drm_encoder *encoder) -{ - struct atmel_hlcdc_rgb_output *rgb = - drm_encoder_to_atmel_hlcdc_rgb_output(encoder); - - if (rgb->panel) { - drm_panel_disable(rgb->panel); - drm_panel_unprepare(rgb->panel); - } -} - -static const struct drm_encoder_helper_funcs atmel_hlcdc_panel_encoder_helper_funcs = { - .disable = atmel_hlcdc_rgb_encoder_disable, - .enable = atmel_hlcdc_rgb_encoder_enable, -}; - static const struct drm_encoder_funcs atmel_hlcdc_panel_encoder_funcs = { .destroy = drm_encoder_cleanup, };
-static int atmel_hlcdc_panel_get_modes(struct drm_connector *connector) -{ - struct atmel_hlcdc_rgb_output *rgb = - drm_connector_to_atmel_hlcdc_rgb_output(connector); - - if (rgb->panel) - return rgb->panel->funcs->get_modes(rgb->panel); - - return 0; -} - -static int atmel_hlcdc_rgb_mode_valid(struct drm_connector *connector, - struct drm_display_mode *mode) -{ - struct atmel_hlcdc_rgb_output *rgb = - drm_connector_to_atmel_hlcdc_rgb_output(connector); - - return atmel_hlcdc_dc_mode_valid(rgb->dc, mode); -} - -static const struct drm_connector_helper_funcs atmel_hlcdc_panel_connector_helper_funcs = { - .get_modes = atmel_hlcdc_panel_get_modes, - .mode_valid = atmel_hlcdc_rgb_mode_valid, -}; - -static enum drm_connector_status -atmel_hlcdc_panel_connector_detect(struct drm_connector *connector, bool force) -{ - struct atmel_hlcdc_rgb_output *rgb = - drm_connector_to_atmel_hlcdc_rgb_output(connector); - - if (rgb->panel) - return connector_status_connected; - - return connector_status_disconnected; -} - -static void -atmel_hlcdc_panel_connector_destroy(struct drm_connector *connector) -{ - struct atmel_hlcdc_rgb_output *rgb = - drm_connector_to_atmel_hlcdc_rgb_output(connector); - - if (rgb->panel) - drm_panel_detach(rgb->panel); - - drm_connector_cleanup(connector); -} - -static const struct drm_connector_funcs atmel_hlcdc_panel_connector_funcs = { - .dpms = drm_atomic_helper_connector_dpms, - .detect = atmel_hlcdc_panel_connector_detect, - .fill_modes = drm_helper_probe_single_connector_modes, - .destroy = atmel_hlcdc_panel_connector_destroy, - .reset = drm_atomic_helper_connector_reset, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, - .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, -}; - static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, const struct device_node *np) { - struct atmel_hlcdc_dc *dc = dev->dev_private; - struct atmel_hlcdc_rgb_output *output; + struct drm_encoder *encoder; struct drm_panel *panel; struct drm_bridge *bridge; int ret;
- output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL); - if (!output) + encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL); + if (!encoder) return -EINVAL;
- output->dc = dc; - - drm_encoder_helper_add(&output->encoder, - &atmel_hlcdc_panel_encoder_helper_funcs); - ret = drm_encoder_init(dev, &output->encoder, + ret = drm_encoder_init(dev, encoder, &atmel_hlcdc_panel_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL); if (ret) return ret;
- output->encoder.possible_crtcs = 0x1; + encoder->possible_crtcs = 0x1;
ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); if (ret) return ret;
if (panel) { - output->connector.dpms = DRM_MODE_DPMS_OFF; - output->connector.polled = DRM_CONNECTOR_POLL_CONNECT; - drm_connector_helper_add(&output->connector, - &atmel_hlcdc_panel_connector_helper_funcs); - ret = drm_connector_init(dev, &output->connector, - &atmel_hlcdc_panel_connector_funcs, - DRM_MODE_CONNECTOR_Unknown); - if (ret) - goto err_encoder_cleanup; - - drm_mode_connector_attach_encoder(&output->connector, - &output->encoder); - - ret = drm_panel_attach(panel, &output->connector); - if (ret) { - drm_connector_cleanup(&output->connector); - goto err_encoder_cleanup; - } - - output->panel = panel; + bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown); + if (IS_ERR(bridge)) + return PTR_ERR(bridge);
return 0; }
if (bridge) { - ret = drm_bridge_attach(&output->encoder, bridge, NULL); + ret = drm_bridge_attach(encoder, bridge, NULL); if (!ret) return 0; + + if (panel) + drm_panel_bridge_remove(bridge); }
-err_encoder_cleanup: - drm_encoder_cleanup(&output->encoder); + drm_encoder_cleanup(encoder);
return ret; }
Hi Eric,
On Thu, 11 May 2017 11:31:28 -0700 Eric Anholt eric@anholt.net wrote:
This cuts 135 lines of boilerplate, at the cost of losing the filtering of get_modes() using atmel_hlcdc_dc_mode_valid(). The atomic check will still check that we don't set an invalid mode, though.
Nice.
Signed-off-by: Eric Anholt eric@anholt.net
Acked-by: Boris Brezillon boris.brezillon@free-electrons.com
A few comments below (no need to address them, those are just minor things that can be fixed later on, or things I'm not comfortable with but cannot be addressed easily).
This patch is just a proposal for atmel-hlcdc if you like it -- maybe you still want the filtering of get_modes(). I'm not sure from a userspace API perspective if we should really have the encoder (your CRTC's limits) filtering modes from a connector, but I'm also pretty sure it doesn't actually matter.
Actually I added this check because someone suggested it :-). I don't think it's a real problem, and that should definitely be checked at the CRTC level, not here.
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 163 ++--------------------- 1 file changed, 14 insertions(+), 149 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c index 4b2cfbd0d43f..340ef962aa81 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c @@ -23,191 +23,56 @@
#include <drm/drmP.h> #include <drm/drm_of.h> +#include <drm/drm_bridge.h>
#include "atmel_hlcdc_dc.h"
-/**
- Atmel HLCDC RGB connector structure
- This structure stores RGB slave device information.
- @connector: DRM connector
- @encoder: DRM encoder
- @dc: pointer to the atmel_hlcdc_dc structure
- @panel: panel connected on the RGB output
- */
-struct atmel_hlcdc_rgb_output {
- struct drm_connector connector;
- struct drm_encoder encoder;
- struct atmel_hlcdc_dc *dc;
- struct drm_panel *panel;
-};
-static inline struct atmel_hlcdc_rgb_output * -drm_connector_to_atmel_hlcdc_rgb_output(struct drm_connector *connector) -{
- return container_of(connector, struct atmel_hlcdc_rgb_output,
connector);-}
-static inline struct atmel_hlcdc_rgb_output * -drm_encoder_to_atmel_hlcdc_rgb_output(struct drm_encoder *encoder) -{
- return container_of(encoder, struct atmel_hlcdc_rgb_output, encoder);
-}
-static void atmel_hlcdc_rgb_encoder_enable(struct drm_encoder *encoder) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_encoder_to_atmel_hlcdc_rgb_output(encoder);- if (rgb->panel) {
drm_panel_prepare(rgb->panel);drm_panel_enable(rgb->panel);- }
-}
-static void atmel_hlcdc_rgb_encoder_disable(struct drm_encoder *encoder) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_encoder_to_atmel_hlcdc_rgb_output(encoder);- if (rgb->panel) {
drm_panel_disable(rgb->panel);drm_panel_unprepare(rgb->panel);- }
-}
-static const struct drm_encoder_helper_funcs atmel_hlcdc_panel_encoder_helper_funcs = {
- .disable = atmel_hlcdc_rgb_encoder_disable,
- .enable = atmel_hlcdc_rgb_encoder_enable,
-};
static const struct drm_encoder_funcs atmel_hlcdc_panel_encoder_funcs = { .destroy = drm_encoder_cleanup, };
-static int atmel_hlcdc_panel_get_modes(struct drm_connector *connector) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
return rgb->panel->funcs->get_modes(rgb->panel);- return 0;
-}
-static int atmel_hlcdc_rgb_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)-{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- return atmel_hlcdc_dc_mode_valid(rgb->dc, mode);
-}
-static const struct drm_connector_helper_funcs atmel_hlcdc_panel_connector_helper_funcs = {
- .get_modes = atmel_hlcdc_panel_get_modes,
- .mode_valid = atmel_hlcdc_rgb_mode_valid,
-};
-static enum drm_connector_status -atmel_hlcdc_panel_connector_detect(struct drm_connector *connector, bool force) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
return connector_status_connected;- return connector_status_disconnected;
-}
-static void -atmel_hlcdc_panel_connector_destroy(struct drm_connector *connector) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
drm_panel_detach(rgb->panel);- drm_connector_cleanup(connector);
-}
-static const struct drm_connector_funcs atmel_hlcdc_panel_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .detect = atmel_hlcdc_panel_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = atmel_hlcdc_panel_connector_destroy,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, const struct device_node *np) {
- struct atmel_hlcdc_dc *dc = dev->dev_private;
- struct atmel_hlcdc_rgb_output *output;
- struct drm_encoder *encoder; struct drm_panel *panel; struct drm_bridge *bridge; int ret;
- output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL);
- if (!output)
- encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
- if (!encoder) return -EINVAL;
- output->dc = dc;
- drm_encoder_helper_add(&output->encoder,
&atmel_hlcdc_panel_encoder_helper_funcs);- ret = drm_encoder_init(dev, &output->encoder,
- ret = drm_encoder_init(dev, encoder, &atmel_hlcdc_panel_encoder_funcs,
atmel_hlcdc_panel_encoder_funcs should probably be renamed atmel_hlcdc_dpi_encoder_funcs, but it's just a detail.
DRM_MODE_ENCODER_NONE, NULL);if (ret) return ret;
- output->encoder.possible_crtcs = 0x1;
encoder->possible_crtcs = 0x1;
ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); if (ret) return ret;
if (panel) {
output->connector.dpms = DRM_MODE_DPMS_OFF;output->connector.polled = DRM_CONNECTOR_POLL_CONNECT;drm_connector_helper_add(&output->connector,&atmel_hlcdc_panel_connector_helper_funcs);ret = drm_connector_init(dev, &output->connector,&atmel_hlcdc_panel_connector_funcs,DRM_MODE_CONNECTOR_Unknown);if (ret)goto err_encoder_cleanup;drm_mode_connector_attach_encoder(&output->connector,&output->encoder);ret = drm_panel_attach(panel, &output->connector);if (ret) {drm_connector_cleanup(&output->connector);goto err_encoder_cleanup;}output->panel = panel;
bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown);if (IS_ERR(bridge))return PTR_ERR(bridge);
It would even be simpler if panels were directly registering themselves to the bridge infrastructure so that they can be found with drm_of_find_bridge() and we don't have to explicitly create this bride->panel wrapper, but I remember that Thierry was not a big fan of this idea.
Also, I keep thinking that we're abusing objects. A bridge is supposed to convert a pixel stream in a given format/encoding into a different format/encoding. Here, there's no conversion taking place in this panel_bridge, it's just a way to expose the panel as a generic element that can be easily connected to many display controller drivers. To be perfectly clear, I really like this idea of providing a generic wrapper, I'm just unsure exposing this wrapper as a bridge is such a good idea, unless we clarify the meaning/goal of a 'drm_bridge' object.
The more I look at it, the more I think the drm_bridge is turning into a generic element that is taking a stream of pixel in a specific format and either consuming it of transforming it before forwarding it to the next element in the chain.
Regards,
Boris
On Thu, May 11, 2017 at 09:15:23PM +0200, Boris Brezillon wrote:
Hi Eric,
On Thu, 11 May 2017 11:31:28 -0700 Eric Anholt eric@anholt.net wrote:
This cuts 135 lines of boilerplate, at the cost of losing the filtering of get_modes() using atmel_hlcdc_dc_mode_valid(). The atomic check will still check that we don't set an invalid mode, though.
Nice.
Signed-off-by: Eric Anholt eric@anholt.net
Acked-by: Boris Brezillon boris.brezillon@free-electrons.com
A few comments below (no need to address them, those are just minor things that can be fixed later on, or things I'm not comfortable with but cannot be addressed easily).
This patch is just a proposal for atmel-hlcdc if you like it -- maybe you still want the filtering of get_modes(). I'm not sure from a userspace API perspective if we should really have the encoder (your CRTC's limits) filtering modes from a connector, but I'm also pretty sure it doesn't actually matter.
Actually I added this check because someone suggested it :-). I don't think it's a real problem, and that should definitely be checked at the CRTC level, not here.
Also note that we're getting ready to merge an entire pile of ->mode_valid callbacks (from Jose Abreu), that should simplify this all a lot. With that we could stuff the mode_valid code either into the encoder or the crtc. -Daniel
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 163 ++--------------------- 1 file changed, 14 insertions(+), 149 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c index 4b2cfbd0d43f..340ef962aa81 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c @@ -23,191 +23,56 @@
#include <drm/drmP.h> #include <drm/drm_of.h> +#include <drm/drm_bridge.h>
#include "atmel_hlcdc_dc.h"
-/**
- Atmel HLCDC RGB connector structure
- This structure stores RGB slave device information.
- @connector: DRM connector
- @encoder: DRM encoder
- @dc: pointer to the atmel_hlcdc_dc structure
- @panel: panel connected on the RGB output
- */
-struct atmel_hlcdc_rgb_output {
- struct drm_connector connector;
- struct drm_encoder encoder;
- struct atmel_hlcdc_dc *dc;
- struct drm_panel *panel;
-};
-static inline struct atmel_hlcdc_rgb_output * -drm_connector_to_atmel_hlcdc_rgb_output(struct drm_connector *connector) -{
- return container_of(connector, struct atmel_hlcdc_rgb_output,
connector);-}
-static inline struct atmel_hlcdc_rgb_output * -drm_encoder_to_atmel_hlcdc_rgb_output(struct drm_encoder *encoder) -{
- return container_of(encoder, struct atmel_hlcdc_rgb_output, encoder);
-}
-static void atmel_hlcdc_rgb_encoder_enable(struct drm_encoder *encoder) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_encoder_to_atmel_hlcdc_rgb_output(encoder);- if (rgb->panel) {
drm_panel_prepare(rgb->panel);drm_panel_enable(rgb->panel);- }
-}
-static void atmel_hlcdc_rgb_encoder_disable(struct drm_encoder *encoder) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_encoder_to_atmel_hlcdc_rgb_output(encoder);- if (rgb->panel) {
drm_panel_disable(rgb->panel);drm_panel_unprepare(rgb->panel);- }
-}
-static const struct drm_encoder_helper_funcs atmel_hlcdc_panel_encoder_helper_funcs = {
- .disable = atmel_hlcdc_rgb_encoder_disable,
- .enable = atmel_hlcdc_rgb_encoder_enable,
-};
static const struct drm_encoder_funcs atmel_hlcdc_panel_encoder_funcs = { .destroy = drm_encoder_cleanup, };
-static int atmel_hlcdc_panel_get_modes(struct drm_connector *connector) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
return rgb->panel->funcs->get_modes(rgb->panel);- return 0;
-}
-static int atmel_hlcdc_rgb_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)-{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- return atmel_hlcdc_dc_mode_valid(rgb->dc, mode);
-}
-static const struct drm_connector_helper_funcs atmel_hlcdc_panel_connector_helper_funcs = {
- .get_modes = atmel_hlcdc_panel_get_modes,
- .mode_valid = atmel_hlcdc_rgb_mode_valid,
-};
-static enum drm_connector_status -atmel_hlcdc_panel_connector_detect(struct drm_connector *connector, bool force) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
return connector_status_connected;- return connector_status_disconnected;
-}
-static void -atmel_hlcdc_panel_connector_destroy(struct drm_connector *connector) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
drm_panel_detach(rgb->panel);- drm_connector_cleanup(connector);
-}
-static const struct drm_connector_funcs atmel_hlcdc_panel_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .detect = atmel_hlcdc_panel_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = atmel_hlcdc_panel_connector_destroy,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, const struct device_node *np) {
- struct atmel_hlcdc_dc *dc = dev->dev_private;
- struct atmel_hlcdc_rgb_output *output;
- struct drm_encoder *encoder; struct drm_panel *panel; struct drm_bridge *bridge; int ret;
- output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL);
- if (!output)
- encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
- if (!encoder) return -EINVAL;
- output->dc = dc;
- drm_encoder_helper_add(&output->encoder,
&atmel_hlcdc_panel_encoder_helper_funcs);- ret = drm_encoder_init(dev, &output->encoder,
- ret = drm_encoder_init(dev, encoder, &atmel_hlcdc_panel_encoder_funcs,
atmel_hlcdc_panel_encoder_funcs should probably be renamed atmel_hlcdc_dpi_encoder_funcs, but it's just a detail.
DRM_MODE_ENCODER_NONE, NULL);if (ret) return ret;
- output->encoder.possible_crtcs = 0x1;
encoder->possible_crtcs = 0x1;
ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); if (ret) return ret;
if (panel) {
output->connector.dpms = DRM_MODE_DPMS_OFF;output->connector.polled = DRM_CONNECTOR_POLL_CONNECT;drm_connector_helper_add(&output->connector,&atmel_hlcdc_panel_connector_helper_funcs);ret = drm_connector_init(dev, &output->connector,&atmel_hlcdc_panel_connector_funcs,DRM_MODE_CONNECTOR_Unknown);if (ret)goto err_encoder_cleanup;drm_mode_connector_attach_encoder(&output->connector,&output->encoder);ret = drm_panel_attach(panel, &output->connector);if (ret) {drm_connector_cleanup(&output->connector);goto err_encoder_cleanup;}output->panel = panel;
bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown);if (IS_ERR(bridge))return PTR_ERR(bridge);It would even be simpler if panels were directly registering themselves to the bridge infrastructure so that they can be found with drm_of_find_bridge() and we don't have to explicitly create this bride->panel wrapper, but I remember that Thierry was not a big fan of this idea.
Also, I keep thinking that we're abusing objects. A bridge is supposed to convert a pixel stream in a given format/encoding into a different format/encoding. Here, there's no conversion taking place in this panel_bridge, it's just a way to expose the panel as a generic element that can be easily connected to many display controller drivers. To be perfectly clear, I really like this idea of providing a generic wrapper, I'm just unsure exposing this wrapper as a bridge is such a good idea, unless we clarify the meaning/goal of a 'drm_bridge' object.
The more I look at it, the more I think the drm_bridge is turning into a generic element that is taking a stream of pixel in a specific format and either consuming it of transforming it before forwarding it to the next element in the chain.
Regards,
Boris _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi,
When pushing to drm-misc-next, I got a minor merge conflict when dim tried to rebuild drm-tip.
The reason for the conflict was because the following commit was present in drm-tip but not in drm-misc:
https://cgit.freedesktop.org/drm-tip/commit/drivers/gpu/drm/atmel-hlcdc/atme...
I fixed the conflict such that drm-tip is rebuit successfully. I'd however like if Eric and Boris could have a look at atmel_hlcdc_output.c to check if things are okay.
Thanks, Archit
On 05/12/2017 12:01 AM, Eric Anholt wrote:
This cuts 135 lines of boilerplate, at the cost of losing the filtering of get_modes() using atmel_hlcdc_dc_mode_valid(). The atomic check will still check that we don't set an invalid mode, though.
Signed-off-by: Eric Anholt eric@anholt.net
This patch is just a proposal for atmel-hlcdc if you like it -- maybe you still want the filtering of get_modes(). I'm not sure from a userspace API perspective if we should really have the encoder (your CRTC's limits) filtering modes from a connector, but I'm also pretty sure it doesn't actually matter.
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 163 ++--------------------- 1 file changed, 14 insertions(+), 149 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c index 4b2cfbd0d43f..340ef962aa81 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c @@ -23,191 +23,56 @@
#include <drm/drmP.h> #include <drm/drm_of.h> +#include <drm/drm_bridge.h>
#include "atmel_hlcdc_dc.h"
-/**
- Atmel HLCDC RGB connector structure
- This structure stores RGB slave device information.
- @connector: DRM connector
- @encoder: DRM encoder
- @dc: pointer to the atmel_hlcdc_dc structure
- @panel: panel connected on the RGB output
- */
-struct atmel_hlcdc_rgb_output {
- struct drm_connector connector;
- struct drm_encoder encoder;
- struct atmel_hlcdc_dc *dc;
- struct drm_panel *panel;
-};
-static inline struct atmel_hlcdc_rgb_output * -drm_connector_to_atmel_hlcdc_rgb_output(struct drm_connector *connector) -{
- return container_of(connector, struct atmel_hlcdc_rgb_output,
connector);-}
-static inline struct atmel_hlcdc_rgb_output * -drm_encoder_to_atmel_hlcdc_rgb_output(struct drm_encoder *encoder) -{
- return container_of(encoder, struct atmel_hlcdc_rgb_output, encoder);
-}
-static void atmel_hlcdc_rgb_encoder_enable(struct drm_encoder *encoder) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_encoder_to_atmel_hlcdc_rgb_output(encoder);- if (rgb->panel) {
drm_panel_prepare(rgb->panel);drm_panel_enable(rgb->panel);- }
-}
-static void atmel_hlcdc_rgb_encoder_disable(struct drm_encoder *encoder) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_encoder_to_atmel_hlcdc_rgb_output(encoder);- if (rgb->panel) {
drm_panel_disable(rgb->panel);drm_panel_unprepare(rgb->panel);- }
-}
-static const struct drm_encoder_helper_funcs atmel_hlcdc_panel_encoder_helper_funcs = {
- .disable = atmel_hlcdc_rgb_encoder_disable,
- .enable = atmel_hlcdc_rgb_encoder_enable,
-};
static const struct drm_encoder_funcs atmel_hlcdc_panel_encoder_funcs = { .destroy = drm_encoder_cleanup, };
-static int atmel_hlcdc_panel_get_modes(struct drm_connector *connector) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
return rgb->panel->funcs->get_modes(rgb->panel);- return 0;
-}
-static int atmel_hlcdc_rgb_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)-{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- return atmel_hlcdc_dc_mode_valid(rgb->dc, mode);
-}
-static const struct drm_connector_helper_funcs atmel_hlcdc_panel_connector_helper_funcs = {
- .get_modes = atmel_hlcdc_panel_get_modes,
- .mode_valid = atmel_hlcdc_rgb_mode_valid,
-};
-static enum drm_connector_status -atmel_hlcdc_panel_connector_detect(struct drm_connector *connector, bool force) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
return connector_status_connected;- return connector_status_disconnected;
-}
-static void -atmel_hlcdc_panel_connector_destroy(struct drm_connector *connector) -{
- struct atmel_hlcdc_rgb_output *rgb =
drm_connector_to_atmel_hlcdc_rgb_output(connector);- if (rgb->panel)
drm_panel_detach(rgb->panel);- drm_connector_cleanup(connector);
-}
-static const struct drm_connector_funcs atmel_hlcdc_panel_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .detect = atmel_hlcdc_panel_connector_detect,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = atmel_hlcdc_panel_connector_destroy,
- .reset = drm_atomic_helper_connector_reset,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
static int atmel_hlcdc_attach_endpoint(struct drm_device *dev, const struct device_node *np) {
- struct atmel_hlcdc_dc *dc = dev->dev_private;
- struct atmel_hlcdc_rgb_output *output;
- struct drm_encoder *encoder; struct drm_panel *panel; struct drm_bridge *bridge; int ret;
- output = devm_kzalloc(dev->dev, sizeof(*output), GFP_KERNEL);
- if (!output)
- encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
- if (!encoder) return -EINVAL;
- output->dc = dc;
- drm_encoder_helper_add(&output->encoder,
&atmel_hlcdc_panel_encoder_helper_funcs);- ret = drm_encoder_init(dev, &output->encoder,
- ret = drm_encoder_init(dev, encoder, &atmel_hlcdc_panel_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL); if (ret) return ret;
- output->encoder.possible_crtcs = 0x1;
encoder->possible_crtcs = 0x1;
ret = drm_of_find_panel_or_bridge(np, 0, 0, &panel, &bridge); if (ret) return ret;
if (panel) {
output->connector.dpms = DRM_MODE_DPMS_OFF;output->connector.polled = DRM_CONNECTOR_POLL_CONNECT;drm_connector_helper_add(&output->connector,&atmel_hlcdc_panel_connector_helper_funcs);ret = drm_connector_init(dev, &output->connector,&atmel_hlcdc_panel_connector_funcs,DRM_MODE_CONNECTOR_Unknown);if (ret)goto err_encoder_cleanup;drm_mode_connector_attach_encoder(&output->connector,&output->encoder);ret = drm_panel_attach(panel, &output->connector);if (ret) {drm_connector_cleanup(&output->connector);goto err_encoder_cleanup;}output->panel = panel;
bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown);if (IS_ERR(bridge))return PTR_ERR(bridge);return 0; }
if (bridge) {
ret = drm_bridge_attach(&output->encoder, bridge, NULL);
ret = drm_bridge_attach(encoder, bridge, NULL);if (panel)drm_panel_bridge_remove(bridge);-err_encoder_cleanup:
- drm_encoder_cleanup(&output->encoder);
drm_encoder_cleanup(encoder);
return ret;
}
Hi Archit,
On Mon, 5 Jun 2017 21:06:20 +0530 Archit Taneja architt@codeaurora.org wrote:
Hi,
When pushing to drm-misc-next, I got a minor merge conflict when dim tried to rebuild drm-tip.
The reason for the conflict was because the following commit was present in drm-tip but not in drm-misc:
https://cgit.freedesktop.org/drm-tip/commit/drivers/gpu/drm/atmel-hlcdc/atme...
I fixed the conflict such that drm-tip is rebuit successfully. I'd however like if Eric and Boris could have a look at atmel_hlcdc_output.c to check if things are okay.
Where can I find the fix you made to fix this problem?
[...]
if (panel) {
output->connector.dpms = DRM_MODE_DPMS_OFF;output->connector.polled = DRM_CONNECTOR_POLL_CONNECT;drm_connector_helper_add(&output->connector,&atmel_hlcdc_panel_connector_helper_funcs);ret = drm_connector_init(dev, &output->connector,&atmel_hlcdc_panel_connector_funcs,DRM_MODE_CONNECTOR_Unknown);if (ret)goto err_encoder_cleanup;drm_mode_connector_attach_encoder(&output->connector,&output->encoder);ret = drm_panel_attach(panel, &output->connector);if (ret) {drm_connector_cleanup(&output->connector);goto err_encoder_cleanup;}output->panel = panel;
bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown);if (IS_ERR(bridge))return PTR_ERR(bridge);return 0;
Just noticed that this is wrong. We should not return here, otherwise the panel bridge is never attached to the encoder.
}
if (bridge) {
ret = drm_bridge_attach(&output->encoder, bridge, NULL);
ret = drm_bridge_attach(encoder, bridge, NULL);if (panel)drm_panel_bridge_remove(bridge);-err_encoder_cleanup:
- drm_encoder_cleanup(&output->encoder);
drm_encoder_cleanup(encoder);
return ret;
}
On 06/06/2017 02:27 PM, Boris Brezillon wrote:
Hi Archit,
On Mon, 5 Jun 2017 21:06:20 +0530 Archit Taneja architt@codeaurora.org wrote:
Hi,
When pushing to drm-misc-next, I got a minor merge conflict when dim tried to rebuild drm-tip.
The reason for the conflict was because the following commit was present in drm-tip but not in drm-misc:
https://cgit.freedesktop.org/drm-tip/commit/drivers/gpu/drm/atmel-hlcdc/atme...
I fixed the conflict such that drm-tip is rebuit successfully. I'd however like if Eric and Boris could have a look at atmel_hlcdc_output.c to check if things are okay.
Where can I find the fix you made to fix this problem?
+Sean, I missed adding him in the original message.
The fix is a part of the commit that merged drm-misc-next into drm-tip:
https://cgit.freedesktop.org/drm-tip/commit/?id=62a258c9c517c7c97bc4545f77af...
The commit "drm/atmel-hlcdc: Fix output initialization" was a part of 4.12-rc2. drm-misc-next is still based off 4.12-rc1, so this commit wasn't present.
Sean will observe some minor merge conflicts when he rebases drm-misc-next onto a more recent tag from Linus.
Thanks, Archit
[...]
if (panel) {
output->connector.dpms = DRM_MODE_DPMS_OFF;output->connector.polled = DRM_CONNECTOR_POLL_CONNECT;drm_connector_helper_add(&output->connector,&atmel_hlcdc_panel_connector_helper_funcs);ret = drm_connector_init(dev, &output->connector,&atmel_hlcdc_panel_connector_funcs,DRM_MODE_CONNECTOR_Unknown);if (ret)goto err_encoder_cleanup;drm_mode_connector_attach_encoder(&output->connector,&output->encoder);ret = drm_panel_attach(panel, &output->connector);if (ret) {drm_connector_cleanup(&output->connector);goto err_encoder_cleanup;}output->panel = panel;
bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_Unknown);if (IS_ERR(bridge))return PTR_ERR(bridge);return 0;
Just noticed that this is wrong. We should not return here, otherwise the panel bridge is never attached to the encoder.
}
if (bridge) {
ret = drm_bridge_attach(&output->encoder, bridge, NULL);
ret = drm_bridge_attach(encoder, bridge, NULL);if (panel)drm_panel_bridge_remove(bridge);-err_encoder_cleanup:
- drm_encoder_cleanup(&output->encoder);
drm_encoder_cleanup(encoder);
return ret;
}
On Thu, 11 May 2017 11:31:22 -0700 Eric Anholt eric@anholt.net wrote:
Many DRM drivers have common code to make a stub connector implementation that wraps a drm_panel. By wrapping the panel in a DRM bridge, all of the connector code (including calls during encoder enable/disable) goes away.
v2: Fix build with CONFIG_DRM=m, drop "dev" argument that should just be the panel's dev, move kerneldoc up a level and document _remove().
Signed-off-by: Eric Anholt eric@anholt.net Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
Reviewed-by: Boris Brezillon boris.brezillon@free-electrons.com
Documentation/gpu/drm-kms-helpers.rst | 6 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/bridge/Kconfig | 11 +- drivers/gpu/drm/bridge/lvds-encoder.c | 157 ++++----------------------- drivers/gpu/drm/bridge/panel.c | 197 ++++++++++++++++++++++++++++++++++ include/drm/drm_bridge.h | 7 ++ 6 files changed, 238 insertions(+), 141 deletions(-) create mode 100644 drivers/gpu/drm/bridge/panel.c
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index c075aadd7078..7c5e2549a58a 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -143,6 +143,12 @@ Bridge Helper Reference .. kernel-doc:: drivers/gpu/drm/drm_bridge.c :export:
+Panel-Bridge Helper Reference +-----------------------------
+.. kernel-doc:: drivers/gpu/drm/bridge/panel.c
- :export:
.. _drm_panel_helper:
Panel Helper Reference diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index c156fecfb362..4cc9c02cc3f2 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -24,6 +24,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm-$(CONFIG_PCI) += ati_pcigart.o drm-$(CONFIG_DRM_PANEL) += drm_panel.o +drm-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm-$(CONFIG_OF) += drm_of.o drm-$(CONFIG_AGP) += drm_agpsupport.o drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index f6968d3b4b41..c4daca38743c 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -4,6 +4,14 @@ config DRM_BRIDGE help Bridge registration and lookup framework.
+config DRM_PANEL_BRIDGE
- def_bool y
- depends on DRM_BRIDGE
- select DRM_KMS_HELPER
- select DRM_PANEL
- help
DRM bridge wrapper of DRM panelsmenu "Display Interface Bridges" depends on DRM && DRM_BRIDGE
@@ -27,8 +35,7 @@ config DRM_DUMB_VGA_DAC config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF
- select DRM_KMS_HELPER
- select DRM_PANEL
- select DRM_PANEL_BRIDGE help Support for transparent parallel to LVDS encoders that don't require any configuration.
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c index f1f67a279426..0903ba574f61 100644 --- a/drivers/gpu/drm/bridge/lvds-encoder.c +++ b/drivers/gpu/drm/bridge/lvds-encoder.c @@ -8,144 +8,18 @@ */
#include <drm/drmP.h> -#include <drm/drm_atomic_helper.h> -#include <drm/drm_connector.h> -#include <drm/drm_crtc_helper.h> -#include <drm/drm_encoder.h> -#include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_bridge.h> #include <drm/drm_panel.h>
#include <linux/of_graph.h>
-struct lvds_encoder {
- struct device *dev;
- struct drm_bridge bridge;
- struct drm_connector connector;
- struct drm_panel *panel;
-};
-static inline struct lvds_encoder * -drm_bridge_to_lvds_encoder(struct drm_bridge *bridge) -{
- return container_of(bridge, struct lvds_encoder, bridge);
-}
-static inline struct lvds_encoder * -drm_connector_to_lvds_encoder(struct drm_connector *connector) -{
- return container_of(connector, struct lvds_encoder, connector);
-}
-static int lvds_connector_get_modes(struct drm_connector *connector) -{
- struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector);
- return drm_panel_get_modes(lvds->panel);
-}
-static const struct drm_connector_helper_funcs lvds_connector_helper_funcs = {
- .get_modes = lvds_connector_get_modes,
-};
-static const struct drm_connector_funcs lvds_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-static int lvds_encoder_attach(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- struct drm_connector *connector = &lvds->connector;
- int ret;
- if (!bridge->encoder) {
DRM_ERROR("Missing encoder\n");return -ENODEV;- }
- drm_connector_helper_add(connector, &lvds_connector_helper_funcs);
- ret = drm_connector_init(bridge->dev, connector, &lvds_connector_funcs,
DRM_MODE_CONNECTOR_LVDS);- if (ret) {
DRM_ERROR("Failed to initialize connector\n");return ret;- }
- drm_mode_connector_attach_encoder(&lvds->connector, bridge->encoder);
- ret = drm_panel_attach(lvds->panel, &lvds->connector);
- if (ret < 0)
return ret;- return 0;
-}
-static void lvds_encoder_detach(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_detach(lvds->panel);
-}
-static void lvds_encoder_pre_enable(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_prepare(lvds->panel);
-}
-static void lvds_encoder_enable(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_enable(lvds->panel);
-}
-static void lvds_encoder_disable(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_disable(lvds->panel);
-}
-static void lvds_encoder_post_disable(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_unprepare(lvds->panel);
-}
-static const struct drm_bridge_funcs lvds_encoder_bridge_funcs = {
- .attach = lvds_encoder_attach,
- .detach = lvds_encoder_detach,
- .pre_enable = lvds_encoder_pre_enable,
- .enable = lvds_encoder_enable,
- .disable = lvds_encoder_disable,
- .post_disable = lvds_encoder_post_disable,
-};
static int lvds_encoder_probe(struct platform_device *pdev) {
- struct lvds_encoder *lvds; struct device_node *port; struct device_node *endpoint;
- struct device_node *panel;
- lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
- if (!lvds)
return -ENOMEM;- lvds->dev = &pdev->dev;
- platform_set_drvdata(pdev, lvds);
- lvds->bridge.funcs = &lvds_encoder_bridge_funcs;
- lvds->bridge.of_node = pdev->dev.of_node;
struct device_node *panel_node;
struct drm_panel *panel;
struct drm_bridge *bridge;
/* Locate the panel DT node. */ port = of_graph_get_port_by_id(pdev->dev.of_node, 1);
@@ -161,29 +35,34 @@ static int lvds_encoder_probe(struct platform_device *pdev) return -ENXIO; }
- panel = of_graph_get_remote_port_parent(endpoint);
- panel_node = of_graph_get_remote_port_parent(endpoint); of_node_put(endpoint);
- if (!panel) {
- if (!panel_node) { dev_dbg(&pdev->dev, "no remote endpoint for port 1\n"); return -ENXIO; }
- lvds->panel = of_drm_find_panel(panel);
- of_node_put(panel);
- if (!lvds->panel) {
- panel = of_drm_find_panel(panel_node);
- of_node_put(panel_node);
- if (!panel) { dev_dbg(&pdev->dev, "panel not found, deferring probe\n"); return -EPROBE_DEFER; }
- /* Register the bridge. */
- return drm_bridge_add(&lvds->bridge);
- bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS);
- if (IS_ERR(bridge))
return PTR_ERR(bridge);- platform_set_drvdata(pdev, bridge);
- return 0;
}
static int lvds_encoder_remove(struct platform_device *pdev) {
- struct lvds_encoder *encoder = platform_get_drvdata(pdev);
- struct drm_bridge *bridge = platform_get_drvdata(pdev);
- drm_bridge_remove(&encoder->bridge);
drm_bridge_remove(bridge);
return 0;
} diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c new file mode 100644 index 000000000000..fd1a78cb1205 --- /dev/null +++ b/drivers/gpu/drm/bridge/panel.c @@ -0,0 +1,197 @@ +/*
- Copyright (C) 2016 Laurent Pinchart laurent.pinchart@ideasonboard.com
- Copyright (C) 2017 Broadcom
- 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_panel.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_connector.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder.h> +#include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_panel.h>
+struct panel_bridge {
- struct drm_bridge bridge;
- struct drm_connector connector;
- struct drm_panel *panel;
- u32 connector_type;
+};
+static inline struct panel_bridge * +drm_bridge_to_panel_bridge(struct drm_bridge *bridge) +{
- return container_of(bridge, struct panel_bridge, bridge);
+}
+static inline struct panel_bridge * +drm_connector_to_panel_bridge(struct drm_connector *connector) +{
- return container_of(connector, struct panel_bridge, connector);
+}
+static int panel_bridge_connector_get_modes(struct drm_connector *connector) +{
- struct panel_bridge *panel_bridge =
drm_connector_to_panel_bridge(connector);- return drm_panel_get_modes(panel_bridge->panel);
+}
+static const struct drm_connector_helper_funcs panel_bridge_connector_helper_funcs = {
- .get_modes = panel_bridge_connector_get_modes,
+};
+static const struct drm_connector_funcs panel_bridge_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+static int panel_bridge_attach(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- struct drm_connector *connector = &panel_bridge->connector;
- int ret;
- if (!bridge->encoder) {
DRM_ERROR("Missing encoder\n");return -ENODEV;- }
- drm_connector_helper_add(connector,
&panel_bridge_connector_helper_funcs);- ret = drm_connector_init(bridge->dev, connector,
&panel_bridge_connector_funcs,panel_bridge->connector_type);- if (ret) {
DRM_ERROR("Failed to initialize connector\n");return ret;- }
- drm_mode_connector_attach_encoder(&panel_bridge->connector,
bridge->encoder);- ret = drm_panel_attach(panel_bridge->panel, &panel_bridge->connector);
- if (ret < 0)
return ret;- return 0;
+}
+static void panel_bridge_detach(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_detach(panel_bridge->panel);
+}
+static void panel_bridge_pre_enable(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_prepare(panel_bridge->panel);
+}
+static void panel_bridge_enable(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_enable(panel_bridge->panel);
+}
+static void panel_bridge_disable(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_disable(panel_bridge->panel);
+}
+static void panel_bridge_post_disable(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_unprepare(panel_bridge->panel);
+}
+static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
- .attach = panel_bridge_attach,
- .detach = panel_bridge_detach,
- .pre_enable = panel_bridge_pre_enable,
- .enable = panel_bridge_enable,
- .disable = panel_bridge_disable,
- .post_disable = panel_bridge_post_disable,
+};
+/**
- drm_panel_bridge_add - Creates a drm_bridge and drm_connector that
- just calls the appropriate functions from drm_panel.
- @panel: The drm_panel being wrapped. Must be non-NULL.
- @connector_type: The DRM_MODE_CONNECTOR_* for the connector to be
- created.
- For drivers converting from directly using drm_panel: The expected
- usage pattern is that during either encoder module probe or DSI
- host attach, a drm_panel will be looked up through
- drm_of_find_panel_or_bridge(). drm_panel_bridge_add() is used to
- wrap that panel in the new bridge, and the result can then be
- passed to drm_bridge_attach(). The drm_panel_prepare() and related
- functions can be dropped from the encoder driver (they're now
- called by the KMS helpers before calling into the encoder), along
- with connector creation. When done with the bridge,
- drm_bridge_detach() should be called as normal, then
- drm_panel_bridge_remove() to free it.
- */
+struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
u32 connector_type)+{
- struct panel_bridge *panel_bridge;
- int ret;
- if (!panel)
return ERR_PTR(EINVAL);- panel_bridge = devm_kzalloc(panel->dev, sizeof(*panel_bridge),
GFP_KERNEL);- if (!panel_bridge)
return ERR_PTR(-ENOMEM);- panel_bridge->connector_type = connector_type;
- panel_bridge->panel = panel;
- panel_bridge->bridge.funcs = &panel_bridge_bridge_funcs;
- panel_bridge->bridge.of_node = panel->dev->of_node;
- ret = drm_bridge_add(&panel_bridge->bridge);
- if (ret)
return ERR_PTR(ret);- return &panel_bridge->bridge;
+} +EXPORT_SYMBOL(drm_panel_bridge_add);
+/**
- drm_panel_bridge_remove - Unregisters and frees a drm_bridge
- created by drm_panel_bridge_add().
- @bridge: The drm_bridge being freed.
- */
+void drm_panel_bridge_remove(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_bridge_remove(bridge);
- devm_kfree(panel_bridge->panel->dev, bridge);
+} +EXPORT_SYMBOL(drm_panel_bridge_remove); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index fdd82fcbf168..bad2178ea3e0 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -29,6 +29,7 @@ #include <drm/drm_modes.h>
struct drm_bridge; +struct drm_panel;
/**
- struct drm_bridge_funcs - drm_bridge control functions
@@ -221,4 +222,10 @@ void drm_bridge_mode_set(struct drm_bridge *bridge, void drm_bridge_pre_enable(struct drm_bridge *bridge); void drm_bridge_enable(struct drm_bridge *bridge);
+#ifdef CONFIG_DRM_PANEL +struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
u32 connector_type);+void drm_panel_bridge_remove(struct drm_bridge *bridge); +#endif
#endif
Hi,
On 05/12/2017 12:01 AM, Eric Anholt wrote:
Many DRM drivers have common code to make a stub connector implementation that wraps a drm_panel. By wrapping the panel in a DRM bridge, all of the connector code (including calls during encoder enable/disable) goes away.
v2: Fix build with CONFIG_DRM=m, drop "dev" argument that should just be the panel's dev, move kerneldoc up a level and document _remove().
Signed-off-by: Eric Anholt eric@anholt.net Acked-by: Daniel Vetter daniel.vetter@ffwll.ch
Documentation/gpu/drm-kms-helpers.rst | 6 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/bridge/Kconfig | 11 +- drivers/gpu/drm/bridge/lvds-encoder.c | 157 ++++----------------------- drivers/gpu/drm/bridge/panel.c | 197 ++++++++++++++++++++++++++++++++++ include/drm/drm_bridge.h | 7 ++ 6 files changed, 238 insertions(+), 141 deletions(-) create mode 100644 drivers/gpu/drm/bridge/panel.c
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index c075aadd7078..7c5e2549a58a 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -143,6 +143,12 @@ Bridge Helper Reference .. kernel-doc:: drivers/gpu/drm/drm_bridge.c :export:
+Panel-Bridge Helper Reference +-----------------------------
+.. kernel-doc:: drivers/gpu/drm/bridge/panel.c
- :export:
.. _drm_panel_helper:
Panel Helper Reference diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index c156fecfb362..4cc9c02cc3f2 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -24,6 +24,7 @@ drm-$(CONFIG_COMPAT) += drm_ioc32.o drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o drm-$(CONFIG_PCI) += ati_pcigart.o drm-$(CONFIG_DRM_PANEL) += drm_panel.o +drm-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o drm-$(CONFIG_OF) += drm_of.o drm-$(CONFIG_AGP) += drm_agpsupport.o drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index f6968d3b4b41..c4daca38743c 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -4,6 +4,14 @@ config DRM_BRIDGE help Bridge registration and lookup framework.
+config DRM_PANEL_BRIDGE
- def_bool y
- depends on DRM_BRIDGE
- select DRM_KMS_HELPER
- select DRM_PANEL
- help
DRM bridge wrapper of DRM panelsmenu "Display Interface Bridges" depends on DRM && DRM_BRIDGE
@@ -27,8 +35,7 @@ config DRM_DUMB_VGA_DAC config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF
- select DRM_KMS_HELPER
- select DRM_PANEL
- select DRM_PANEL_BRIDGE help Support for transparent parallel to LVDS encoders that don't require any configuration.
diff --git a/drivers/gpu/drm/bridge/lvds-encoder.c b/drivers/gpu/drm/bridge/lvds-encoder.c index f1f67a279426..0903ba574f61 100644 --- a/drivers/gpu/drm/bridge/lvds-encoder.c +++ b/drivers/gpu/drm/bridge/lvds-encoder.c @@ -8,144 +8,18 @@ */
#include <drm/drmP.h> -#include <drm/drm_atomic_helper.h> -#include <drm/drm_connector.h> -#include <drm/drm_crtc_helper.h> -#include <drm/drm_encoder.h> -#include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_bridge.h> #include <drm/drm_panel.h>
#include <linux/of_graph.h>
-struct lvds_encoder {
- struct device *dev;
- struct drm_bridge bridge;
- struct drm_connector connector;
- struct drm_panel *panel;
-};
-static inline struct lvds_encoder * -drm_bridge_to_lvds_encoder(struct drm_bridge *bridge) -{
- return container_of(bridge, struct lvds_encoder, bridge);
-}
-static inline struct lvds_encoder * -drm_connector_to_lvds_encoder(struct drm_connector *connector) -{
- return container_of(connector, struct lvds_encoder, connector);
-}
-static int lvds_connector_get_modes(struct drm_connector *connector) -{
- struct lvds_encoder *lvds = drm_connector_to_lvds_encoder(connector);
- return drm_panel_get_modes(lvds->panel);
-}
-static const struct drm_connector_helper_funcs lvds_connector_helper_funcs = {
- .get_modes = lvds_connector_get_modes,
-};
-static const struct drm_connector_funcs lvds_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-static int lvds_encoder_attach(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- struct drm_connector *connector = &lvds->connector;
- int ret;
- if (!bridge->encoder) {
DRM_ERROR("Missing encoder\n");return -ENODEV;- }
- drm_connector_helper_add(connector, &lvds_connector_helper_funcs);
- ret = drm_connector_init(bridge->dev, connector, &lvds_connector_funcs,
DRM_MODE_CONNECTOR_LVDS);- if (ret) {
DRM_ERROR("Failed to initialize connector\n");return ret;- }
- drm_mode_connector_attach_encoder(&lvds->connector, bridge->encoder);
- ret = drm_panel_attach(lvds->panel, &lvds->connector);
- if (ret < 0)
return ret;- return 0;
-}
-static void lvds_encoder_detach(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_detach(lvds->panel);
-}
-static void lvds_encoder_pre_enable(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_prepare(lvds->panel);
-}
-static void lvds_encoder_enable(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_enable(lvds->panel);
-}
-static void lvds_encoder_disable(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_disable(lvds->panel);
-}
-static void lvds_encoder_post_disable(struct drm_bridge *bridge) -{
- struct lvds_encoder *lvds = drm_bridge_to_lvds_encoder(bridge);
- drm_panel_unprepare(lvds->panel);
-}
-static const struct drm_bridge_funcs lvds_encoder_bridge_funcs = {
- .attach = lvds_encoder_attach,
- .detach = lvds_encoder_detach,
- .pre_enable = lvds_encoder_pre_enable,
- .enable = lvds_encoder_enable,
- .disable = lvds_encoder_disable,
- .post_disable = lvds_encoder_post_disable,
-};
static int lvds_encoder_probe(struct platform_device *pdev) {
- struct lvds_encoder *lvds; struct device_node *port; struct device_node *endpoint;
- struct device_node *panel;
- lvds = devm_kzalloc(&pdev->dev, sizeof(*lvds), GFP_KERNEL);
- if (!lvds)
return -ENOMEM;- lvds->dev = &pdev->dev;
- platform_set_drvdata(pdev, lvds);
- lvds->bridge.funcs = &lvds_encoder_bridge_funcs;
- lvds->bridge.of_node = pdev->dev.of_node;
struct device_node *panel_node;
struct drm_panel *panel;
struct drm_bridge *bridge;
/* Locate the panel DT node. */ port = of_graph_get_port_by_id(pdev->dev.of_node, 1);
@@ -161,29 +35,34 @@ static int lvds_encoder_probe(struct platform_device *pdev) return -ENXIO; }
- panel = of_graph_get_remote_port_parent(endpoint);
- panel_node = of_graph_get_remote_port_parent(endpoint); of_node_put(endpoint);
- if (!panel) {
- if (!panel_node) { dev_dbg(&pdev->dev, "no remote endpoint for port 1\n"); return -ENXIO; }
- lvds->panel = of_drm_find_panel(panel);
- of_node_put(panel);
- if (!lvds->panel) {
- panel = of_drm_find_panel(panel_node);
- of_node_put(panel_node);
- if (!panel) { dev_dbg(&pdev->dev, "panel not found, deferring probe\n"); return -EPROBE_DEFER; }
- /* Register the bridge. */
- return drm_bridge_add(&lvds->bridge);
- bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS);
- if (IS_ERR(bridge))
return PTR_ERR(bridge);- platform_set_drvdata(pdev, bridge);
- return 0;
}
static int lvds_encoder_remove(struct platform_device *pdev) {
- struct lvds_encoder *encoder = platform_get_drvdata(pdev);
- struct drm_bridge *bridge = platform_get_drvdata(pdev);
- drm_bridge_remove(&encoder->bridge);
drm_bridge_remove(bridge);
return 0;
} diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c new file mode 100644 index 000000000000..fd1a78cb1205 --- /dev/null +++ b/drivers/gpu/drm/bridge/panel.c @@ -0,0 +1,197 @@ +/*
- Copyright (C) 2016 Laurent Pinchart laurent.pinchart@ideasonboard.com
- Copyright (C) 2017 Broadcom
- 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_panel.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_connector.h> +#include <drm/drm_crtc_helper.h> +#include <drm/drm_encoder.h> +#include <drm/drm_modeset_helper_vtables.h> +#include <drm/drm_panel.h>
+struct panel_bridge {
- struct drm_bridge bridge;
- struct drm_connector connector;
- struct drm_panel *panel;
- u32 connector_type;
+};
+static inline struct panel_bridge * +drm_bridge_to_panel_bridge(struct drm_bridge *bridge) +{
- return container_of(bridge, struct panel_bridge, bridge);
+}
+static inline struct panel_bridge * +drm_connector_to_panel_bridge(struct drm_connector *connector) +{
- return container_of(connector, struct panel_bridge, connector);
+}
+static int panel_bridge_connector_get_modes(struct drm_connector *connector) +{
- struct panel_bridge *panel_bridge =
drm_connector_to_panel_bridge(connector);- return drm_panel_get_modes(panel_bridge->panel);
+}
+static const struct drm_connector_helper_funcs panel_bridge_connector_helper_funcs = {
This can be wrapped to 80 lines to prevent checkpatch warnings.
- .get_modes = panel_bridge_connector_get_modes,
+};
+static const struct drm_connector_funcs panel_bridge_connector_funcs = {
- .dpms = drm_atomic_helper_connector_dpms,
- .reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+static int panel_bridge_attach(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- struct drm_connector *connector = &panel_bridge->connector;
- int ret;
- if (!bridge->encoder) {
DRM_ERROR("Missing encoder\n");return -ENODEV;- }
- drm_connector_helper_add(connector,
&panel_bridge_connector_helper_funcs);- ret = drm_connector_init(bridge->dev, connector,
&panel_bridge_connector_funcs,panel_bridge->connector_type);- if (ret) {
DRM_ERROR("Failed to initialize connector\n");return ret;- }
- drm_mode_connector_attach_encoder(&panel_bridge->connector,
bridge->encoder);- ret = drm_panel_attach(panel_bridge->panel, &panel_bridge->connector);
- if (ret < 0)
return ret;- return 0;
+}
+static void panel_bridge_detach(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_detach(panel_bridge->panel);
+}
+static void panel_bridge_pre_enable(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_prepare(panel_bridge->panel);
+}
+static void panel_bridge_enable(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_enable(panel_bridge->panel);
+}
+static void panel_bridge_disable(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_disable(panel_bridge->panel);
+}
+static void panel_bridge_post_disable(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_panel_unprepare(panel_bridge->panel);
+}
+static const struct drm_bridge_funcs panel_bridge_bridge_funcs = {
- .attach = panel_bridge_attach,
- .detach = panel_bridge_detach,
- .pre_enable = panel_bridge_pre_enable,
- .enable = panel_bridge_enable,
- .disable = panel_bridge_disable,
- .post_disable = panel_bridge_post_disable,
+};
+/**
- drm_panel_bridge_add - Creates a drm_bridge and drm_connector that
- just calls the appropriate functions from drm_panel.
- @panel: The drm_panel being wrapped. Must be non-NULL.
- @connector_type: The DRM_MODE_CONNECTOR_* for the connector to be
- created.
- For drivers converting from directly using drm_panel: The expected
- usage pattern is that during either encoder module probe or DSI
- host attach, a drm_panel will be looked up through
- drm_of_find_panel_or_bridge(). drm_panel_bridge_add() is used to
- wrap that panel in the new bridge, and the result can then be
- passed to drm_bridge_attach(). The drm_panel_prepare() and related
- functions can be dropped from the encoder driver (they're now
- called by the KMS helpers before calling into the encoder), along
- with connector creation. When done with the bridge,
- drm_bridge_detach() should be called as normal, then
- drm_panel_bridge_remove() to free it.
- */
+struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
u32 connector_type)
There are more arguments besides the connector_type that we may require to create the connector correctly. Things like connector's 'bus_formats' and 'polled' fields need to come from the caller. I guess they don't matter as much for simple panels, but we'd probably want to wrap them all in a struct in the future.
+{
- struct panel_bridge *panel_bridge;
- int ret;
- if (!panel)
return ERR_PTR(EINVAL);- panel_bridge = devm_kzalloc(panel->dev, sizeof(*panel_bridge),
GFP_KERNEL);- if (!panel_bridge)
return ERR_PTR(-ENOMEM);- panel_bridge->connector_type = connector_type;
- panel_bridge->panel = panel;
- panel_bridge->bridge.funcs = &panel_bridge_bridge_funcs;
- panel_bridge->bridge.of_node = panel->dev->of_node;
- ret = drm_bridge_add(&panel_bridge->bridge);
- if (ret)
return ERR_PTR(ret);- return &panel_bridge->bridge;
+} +EXPORT_SYMBOL(drm_panel_bridge_add);
+/**
- drm_panel_bridge_remove - Unregisters and frees a drm_bridge
- created by drm_panel_bridge_add().
- @bridge: The drm_bridge being freed.
- */
+void drm_panel_bridge_remove(struct drm_bridge *bridge) +{
- struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
- drm_bridge_remove(bridge);
- devm_kfree(panel_bridge->panel->dev, bridge);
+} +EXPORT_SYMBOL(drm_panel_bridge_remove); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index fdd82fcbf168..bad2178ea3e0 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -29,6 +29,7 @@ #include <drm/drm_modes.h>
struct drm_bridge; +struct drm_panel;
/**
- struct drm_bridge_funcs - drm_bridge control functions
@@ -221,4 +222,10 @@ void drm_bridge_mode_set(struct drm_bridge *bridge, void drm_bridge_pre_enable(struct drm_bridge *bridge); void drm_bridge_enable(struct drm_bridge *bridge);
+#ifdef CONFIG_DRM_PANEL +struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
u32 connector_type);+void drm_panel_bridge_remove(struct drm_bridge *bridge); +#endif
Shouldn't this be CONFIG_DRM_PANEL_BRIDGE?
As I mentioned on irc, I was facing some build issues with this patch applied. Once we sort that out:
Acked-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
dri-devel@lists.freedesktop.org
-
Archit Taneja -
Boris Brezillon -
Daniel Vetter -
Eric Anholt -
Philippe CORNU