MIPI DSI TX subsystem allows you to quickly create systems based on the MIPI protocol. It interfaces between the video processing subsystems and MIPI-based displays. An internal high-speed physical layer design, D-PHY, is provided to allow direct connection to display peripherals.
The subsystem consists of the following sub-blocks: - MIPI D-PHY - MIPI DSI TX Controller - AXI Crossbar
Please refer pg238 [1].
The DSI TX Controller receives stream of image data through an input stream interface. At design time, this subsystem can be configured to number of lanes and pixel format.
This patch series adds the dt-binding and DRM driver for Xilinx DSI-Tx soft IP.
Changes in V2: - Rebased on 5.19 kernel - Moved mode_set functionality to atomic_enable as its deprecrated - Mask fixes - Replaced panel calls with bridge API's - Added mandatory atomic operations - Removed unnecessary logging - Added ARCH_ZYNQMP dependency in Kconfig - Fixed YAML warnings - Cleanup
Venkateshwar Rao Gannavarapu (2): dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation drm: xlnx: dsi: Add Xilinx MIPI DSI-Tx subsystem driver
.../bindings/display/xlnx/xlnx,dsi-tx.yaml | 101 +++++ drivers/gpu/drm/xlnx/Kconfig | 12 + drivers/gpu/drm/xlnx/Makefile | 1 + drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 +++++++++++++++++++++ 4 files changed, 543 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
This patch adds dt binding for Xilinx DSI-TX subsystem.
The Xilinx MIPI DSI (Display serial interface) Transmitter Subsystem implements the Mobile Industry Processor Interface (MIPI) based display interface. It supports the interface with the programmable logic (FPGA).
Signed-off-by: Venkateshwar Rao Gannavarapu venkateshwar.rao.gannavarapu@xilinx.com --- .../bindings/display/xlnx/xlnx,dsi-tx.yaml | 101 +++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml new file mode 100644 index 0000000..644934d --- /dev/null +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml @@ -0,0 +1,101 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi-tx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Xilinx DSI Transmitter subsystem Device Tree Bindings + +maintainers: + - Venkateshwar Rao Gannavarapu venkateshwar.rao.gannavarapu@xilinx.com + +description: | + The Xilinx DSI Transmitter Subsystem implements the Mobile Industry + Processor Interface based display interface. It supports the interface + with the programmable logic (FPGA). + + For more details refer to PG238 Xilinx MIPI DSI-V2.0 Tx Subsystem. + +properties: + compatible: + const: xlnx,dsi-tx-v2.0 + + reg: + maxItems: 1 + + clocks: + items: + - description: AXI Lite CPU clock + - description: D-PHY clock + + clock-names: + items: + - const: s_axis_aclk + - const: dphy_clk_200M + + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + This port should be the input endpoint. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + This port should be the output endpoint. + +required: + - "#address-cells" + - "#size-cells" + - compatible + - reg + - clocks + - clock-names + - ports + +additionalProperties: false + +examples: + - | + dsi0: dsi_tx@80020000 { + compatible = "xlnx,dsi-tx-v2.0"; + reg = <0x80020000 0x20000>; + clocks = <&misc_clk_0>, <&misc_clk_1>; + clock-names = "s_axis_aclk", "dphy_clk_200M"; + #address-cells = <1>; + #size-cells = <0>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + mipi_dsi_in: endpoint { + remote-endpoint = <&pl_disp>; + }; + }; + + port@1 { + reg = <1>; + mipi_dsi_out: endpoint { + remote-endpoint = <&panel_in>; + }; + }; + }; + + panel@0 { + compatible = "auo,b101uan01"; + reg = <0>; + port { + panel_in: endpoint { + remote-endpoint = <&mipi_dsi_out>; + }; + }; + }; + }; + +... -- 1.8.3.1
Hi GVRao,
Thank you for the patch.
On Thu, Jun 16, 2022 at 07:47:35PM +0530, Venkateshwar Rao Gannavarapu wrote:
Port and endpoint are two different things. You can just write "Input port of the DSI encoder".
And here, "Output port of the DSI encoder".
I think those should be listed in the properties above, with fixed values.
Does this example validate (with `make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml`) without listing the panel node in the properties ?
The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video data from AXI-4 stream interface.
It supports upto 4 lanes, optional register interface for the DPHY and multiple RGB color formats. This is a MIPI-DSI host driver and provides DSI bus for panels. This driver also helps to communicate with its panel using panel framework.
Signed-off-by: Venkateshwar Rao Gannavarapu venkateshwar.rao.gannavarapu@xilinx.com --- drivers/gpu/drm/xlnx/Kconfig | 12 ++ drivers/gpu/drm/xlnx/Makefile | 1 + drivers/gpu/drm/xlnx/xlnx_dsi.c | 429 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 442 insertions(+) create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c
diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig index f9cf93c..a75bd76 100644 --- a/drivers/gpu/drm/xlnx/Kconfig +++ b/drivers/gpu/drm/xlnx/Kconfig @@ -1,3 +1,15 @@ +config DRM_XLNX_DSI + tristate "Xilinx DRM DSI Subsystem Driver" + depends on ARCH_ZYNQMP || COMPILE_TEST + depends on DRM && OF + select DRM_KMS_HELPER + select DRM_MIPI_DSI + select DRM_PANEL_BRIDGE + help + DRM bridge driver for Xilinx programmable DSI subsystem controller. + choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in + video pipeline. + config DRM_ZYNQMP_DPSUB tristate "ZynqMP DisplayPort Controller Driver" depends on ARCH_ZYNQMP || COMPILE_TEST diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile index 51c24b7..f90849b 100644 --- a/drivers/gpu/drm/xlnx/Makefile +++ b/drivers/gpu/drm/xlnx/Makefile @@ -1,2 +1,3 @@ +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c new file mode 100644 index 0000000..39d8947 --- /dev/null +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c @@ -0,0 +1,429 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Xilinx FPGA MIPI DSI Tx Controller driver. + * + * Copyright (C) 2022 Xilinx, Inc. + * + * Author: Venkateshwar Rao Gannavarapu venkateshwar.rao.gannavarapu@xilinx.com + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/of_device.h> + +#include <drm/drm_atomic_helper.h> +#include <drm/drm_bridge.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_modes.h> +#include <drm/drm_of.h> +#include <drm/drm_panel.h> +#include <drm/drm_print.h> + +/* DSI Tx IP registers */ +#define XDSI_CCR 0x00 +#define XDSI_CCR_COREENB BIT(0) +#define XDSI_CCR_SOFTRST BIT(1) +#define XDSI_CCR_CRREADY BIT(2) +#define XDSI_CCR_CMDMODE BIT(3) +#define XDSI_CCR_DFIFORST BIT(4) +#define XDSI_CCR_CMDFIFORST BIT(5) +#define XDSI_PCR 0x04 +#define XDSI_PCR_LANES_MASK 3 +#define XDSI_PCR_VIDEOMODE(x) (((x) & 0x3) << 3) +#define XDSI_PCR_VIDEOMODE_MASK GENMASK(4, 3) +#define XDSI_PCR_VIDEOMODE_SHIFT 3 +#define XDSI_PCR_BLLPTYPE(x) ((x) << 5) +#define XDSI_PCR_BLLPMODE(x) ((x) << 6) +#define XDSI_PCR_PIXELFORMAT_MASK GENMASK(12, 11) +#define XDSI_PCR_PIXELFORMAT_SHIFT 11 +#define XDSI_PCR_EOTPENABLE(x) ((x) << 13) +#define XDSI_GIER 0x20 +#define XDSI_ISR 0x24 +#define XDSI_IER 0x28 +#define XDSI_STR 0x2C +#define XDSI_STR_RDY_SHPKT BIT(6) +#define XDSI_STR_RDY_LNGPKT BIT(7) +#define XDSI_STR_DFIFO_FULL BIT(8) +#define XDSI_STR_DFIFO_EMPTY BIT(9) +#define XDSI_STR_WAITFR_DATA BIT(10) +#define XDSI_STR_CMD_EXE_PGS BIT(11) +#define XDSI_STR_CCMD_PROC BIT(12) +#define XDSI_CMD 0x30 +#define XDSI_CMD_QUEUE_PACKET(x) ((x) & GENMASK(23, 0)) +#define XDSI_DFR 0x34 +#define XDSI_TIME1 0x50 +#define XDSI_TIME1_BLLP_BURST(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME1_HSA(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_TIME2 0x54 +#define XDSI_TIME2_VACT(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME2_HACT(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_HACT_MULTIPLIER GENMASK(1, 0) +#define XDSI_TIME3 0x58 +#define XDSI_TIME3_HFP(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME3_HBP(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_TIME4 0x5c +#define XDSI_TIME4_VFP(x) ((x) & GENMASK(7, 0)) +#define XDSI_TIME4_VBP(x) (((x) & GENMASK(7, 0)) << 8) +#define XDSI_TIME4_VSA(x) (((x) & GENMASK(7, 0)) << 16) +#define XDSI_NUM_DATA_T 4 + +/** + * struct xlnx_dsi - Xilinx DSI-TX core + * @bridge: DRM bridge structure + * @dsi_host: DSI host device + * @next_bridge: bridge structure + * @dev: device structure + * @clks: clock source structure + * @iomem: Base address of DSI subsystem + * @mode_flags: DSI operation mode related flags + * @lanes: number of active data lanes supported by DSI controller + * @mul_factor: multiplication factor for HACT timing + * @format: pixel format for video mode of DSI controller + * @device_found: Flag to indicate device presence + */ +struct xlnx_dsi { + struct drm_bridge bridge; + struct mipi_dsi_host dsi_host; + struct drm_bridge *next_bridge; + struct device *dev; + struct clk_bulk_data *clks; + void __iomem *iomem; + unsigned long mode_flags; + u32 lanes; + u32 mul_factor; + enum mipi_dsi_pixel_format format; + bool device_found; +}; + +static const struct clk_bulk_data xdsi_clks[] = { + { .id = "s_axis_aclk" }, + { .id = "dphy_clk_200M" }, +}; + +static inline struct xlnx_dsi *host_to_dsi(struct mipi_dsi_host *host) +{ + return container_of(host, struct xlnx_dsi, dsi_host); +} + +static inline struct xlnx_dsi *bridge_to_dsi(struct drm_bridge *bridge) +{ + return container_of(bridge, struct xlnx_dsi, bridge); +} + +static inline void xlnx_dsi_write(struct xlnx_dsi *dsi, int offset, u32 val) +{ + writel(val, dsi->iomem + offset); +} + +static inline u32 xlnx_dsi_read(struct xlnx_dsi *dsi, int offset) +{ + return readl(dsi->iomem + offset); +} + +static int xlnx_dsi_host_attach(struct mipi_dsi_host *host, + struct mipi_dsi_device *device) +{ + struct xlnx_dsi *dsi = host_to_dsi(host); + struct device *dev = host->dev; + + dsi->mode_flags = device->mode_flags; + + if (dsi->lanes != device->lanes) { + dev_err(dsi->dev, "Mismatch of lanes. panel = %d, DSI = %d\n", + device->lanes, dsi->lanes); + return -EINVAL; + } + + if (dsi->format != device->format) { + dev_err(dsi->dev, "Mismatch of format. panel = %d, DSI = %d\n", + device->format, dsi->format); + return -EINVAL; + } + + if (!dsi->device_found) { + dsi->next_bridge = devm_drm_of_get_bridge(dev, + dev->of_node, 0, 0); + if (IS_ERR(dsi->next_bridge)) + return PTR_ERR(dsi->next_bridge); + drm_bridge_add(&dsi->bridge); + dsi->device_found = true; + } + + return 0; +} + +static int xlnx_dsi_host_detach(struct mipi_dsi_host *host, + struct mipi_dsi_device *device) +{ + struct xlnx_dsi *dsi = host_to_dsi(host); + + drm_bridge_remove(&dsi->bridge); + return 0; +} + +static const struct mipi_dsi_host_ops xlnx_dsi_ops = { + .attach = xlnx_dsi_host_attach, + .detach = xlnx_dsi_host_detach, +}; + +static void +xlnx_dsi_bridge_disable(struct drm_bridge *bridge, + struct drm_bridge_state *old_bridge_state) +{ + struct xlnx_dsi *dsi = bridge_to_dsi(bridge); + u32 reg = xlnx_dsi_read(dsi, XDSI_CCR); + + reg &= ~XDSI_CCR_COREENB; + xlnx_dsi_write(dsi, XDSI_CCR, reg); +} + +static void +xlnx_dsi_bridge_enable(struct drm_bridge *bridge, + struct drm_bridge_state *old_bridge_state) +{ + struct xlnx_dsi *dsi = bridge_to_dsi(bridge); + struct drm_atomic_state *state = old_bridge_state->base.state; + struct drm_connector *connector; + struct drm_crtc *crtc; + const struct drm_crtc_state *crtc_state; + const struct drm_display_mode *mode; + u32 reg, video_mode; + + connector = drm_atomic_get_new_connector_for_encoder(state, + bridge->encoder); + crtc = drm_atomic_get_new_connector_state(state, connector)->crtc; + crtc_state = drm_atomic_get_new_crtc_state(state, crtc); + mode = &crtc_state->adjusted_mode; + + reg = xlnx_dsi_read(dsi, XDSI_PCR); + video_mode = (reg & XDSI_PCR_VIDEOMODE_MASK) >> XDSI_PCR_VIDEOMODE_SHIFT; + + if (!video_mode && (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)) { + reg = XDSI_TIME1_HSA(mode->hsync_end - + mode->hsync_start); + xlnx_dsi_write(dsi, XDSI_TIME1, reg); + } + + reg = XDSI_TIME4_VFP(mode->vsync_start - mode->vdisplay) | + XDSI_TIME4_VBP(mode->vtotal - mode->vsync_end) | + XDSI_TIME4_VSA(mode->vsync_end - mode->vsync_start); + xlnx_dsi_write(dsi, XDSI_TIME4, reg); + + reg = XDSI_TIME3_HFP(mode->hsync_start - mode->hdisplay) | + XDSI_TIME3_HBP(mode->htotal - mode->hsync_end); + xlnx_dsi_write(dsi, XDSI_TIME3, reg); + + reg = XDSI_TIME2_HACT(mode->hdisplay * dsi->mul_factor / 100) | + XDSI_TIME2_VACT(mode->vdisplay); + xlnx_dsi_write(dsi->iomem, XDSI_TIME2, reg); + + xlnx_dsi_write(dsi, XDSI_PCR, XDSI_PCR_VIDEOMODE(BIT(0))); + + /* Enable Core */ + reg = xlnx_dsi_read(dsi, XDSI_CCR); + reg |= XDSI_CCR_COREENB; + xlnx_dsi_write(dsi, XDSI_CCR, reg); +} + +#define MAX_INPUT_SEL_FORMATS 3 +static u32 +*xlnx_dsi_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, + struct drm_bridge_state *bridge_state, + struct drm_crtc_state *crtc_state, + struct drm_connector_state *conn_state, + u32 output_fmt, + unsigned int *num_input_fmts) +{ + u32 *input_fmts; + unsigned int i = 0; + + *num_input_fmts = 0; + input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts), GFP_KERNEL); + if (!input_fmts) + return NULL; + + switch (output_fmt) { + case MEDIA_BUS_FMT_FIXED: + input_fmts[i++] = MEDIA_BUS_FMT_RGB888_1X24; + break; + case MEDIA_BUS_FMT_RGB666_1X18: + input_fmts[i++] = MEDIA_BUS_FMT_RGB666_1X18; + break; + case MEDIA_BUS_FMT_RGB565_1X16: + input_fmts[i++] = MEDIA_BUS_FMT_RGB565_1X16; + break; + default: /* define */ + } + + *num_input_fmts = i; + if (*num_input_fmts == 0) { + kfree(input_fmts); + input_fmts = NULL; + } + + return input_fmts; +} + +static int xlnx_dsi_bridge_attach(struct drm_bridge *bridge, + enum drm_bridge_attach_flags flags) +{ + struct xlnx_dsi *dsi = bridge_to_dsi(bridge); + + if (!dsi->next_bridge) + return 0; + + /* Attach the next bridge */ + return drm_bridge_attach(bridge->encoder, dsi->next_bridge, bridge, + flags); +} + +static void xlnx_dsi_bridge_detach(struct drm_bridge *bridge) +{ + struct xlnx_dsi *dsi = bridge_to_dsi(bridge); + + drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0); +} + +static enum drm_mode_status +xlnx_dsi_bridge_mode_valid(struct drm_bridge *bridge, + const struct drm_display_info *info, + const struct drm_display_mode *mode) +{ + if ((mode->hdisplay & XDSI_HACT_MULTIPLIER) != 0) + return MODE_BAD_WIDTH; + + return MODE_OK; +} + +static const struct drm_bridge_funcs xlnx_dsi_bridge_funcs = { + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, + .atomic_reset = drm_atomic_helper_bridge_reset, + .atomic_disable = xlnx_dsi_bridge_disable, + .atomic_enable = xlnx_dsi_bridge_enable, + .atomic_get_input_bus_fmts = xlnx_dsi_bridge_atomic_get_input_bus_fmts, + .attach = xlnx_dsi_bridge_attach, + .detach = xlnx_dsi_bridge_detach, + .mode_valid = xlnx_dsi_bridge_mode_valid, +}; + +static int xlnx_dsi_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct resource *res; + struct xlnx_dsi *dsi; + int ret; + const int xdsi_mul_fact[XDSI_NUM_DATA_T] = {300, 225, 225, 200}; + u32 reg; + + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL); + if (!dsi) + return -ENOMEM; + + dsi->dev = dev; + dsi->clks = devm_kmemdup(dev, xdsi_clks, sizeof(xdsi_clks), + GFP_KERNEL); + if (!dsi->clks) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + dsi->iomem = devm_ioremap_resource(dev, res); + if (IS_ERR(dsi->iomem)) + return PTR_ERR(dsi->iomem); + + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(xdsi_clks), dsi->clks); + if (ret) + return ret; + + ret = clk_bulk_prepare_enable(ARRAY_SIZE(xdsi_clks), dsi->clks); + if (ret) + return ret; + + platform_set_drvdata(pdev, dsi); + dsi->dsi_host.ops = &xlnx_dsi_ops; + dsi->dsi_host.dev = dev; + + ret = mipi_dsi_host_register(&dsi->dsi_host); + if (ret) { + dev_err(dev, "Failed to register MIPI host: %d\n", ret); + goto err_clk_put; + } + + dsi->bridge.driver_private = dsi; + dsi->bridge.funcs = &xlnx_dsi_bridge_funcs; + dsi->bridge.of_node = pdev->dev.of_node; + + reg = xlnx_dsi_read(dsi, XDSI_PCR); + dsi->lanes = reg & XDSI_PCR_LANES_MASK; + dsi->format = (reg & XDSI_PCR_PIXELFORMAT_MASK) >> + XDSI_PCR_PIXELFORMAT_SHIFT; + + if (dsi->lanes > 4 || dsi->lanes < 1) { + dev_err(dsi->dev, "%d invalid lanes\n", dsi->lanes); + return -EINVAL; + } + + if (dsi->format > MIPI_DSI_FMT_RGB565) { + dev_err(dsi->dev, "Invalid xlnx,dsi-data-type string\n"); + return -EINVAL; + } + + /* + * Used as a multiplication factor for HACT based on used + * DSI data type. + * + * e.g. for RGB666_L datatype and 1920x1080 resolution, + * the Hact (WC) would be as follows - + * 1920 pixels * 18 bits per pixel / 8 bits per byte + * = 1920 pixels * 2.25 bytes per pixel = 4320 bytes. + * + * Data Type - Multiplication factor + * RGB888 - 3 + * RGB666_L - 2.25 +- * RGB666_P - 2.25 + * RGB565 - 2 + * + * Since the multiplication factor is a floating number, + * a 100x multiplication factor is used. + */ + dsi->mul_factor = xdsi_mul_fact[dsi->format]; + + dev_dbg(dsi->dev, "DSI controller num lanes = %d\n", dsi->lanes); + dev_dbg(dsi->dev, "DSI controller format = %d\n", dsi->format); + +err_clk_put: + clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks); + + return ret; +} + +static int xlnx_dsi_remove(struct platform_device *pdev) +{ + struct xlnx_dsi *dsi = platform_get_drvdata(pdev); + + mipi_dsi_host_unregister(&dsi->dsi_host); + clk_bulk_disable_unprepare(ARRAY_SIZE(xdsi_clks), dsi->clks); + + return 0; +} + +static const struct of_device_id xlnx_dsi_of_match[] = { + { .compatible = "xlnx,dsi-tx-v2.0"}, + { } +}; +MODULE_DEVICE_TABLE(of, xlnx_dsi_of_match); + +static struct platform_driver dsi_driver = { + .probe = xlnx_dsi_probe, + .remove = xlnx_dsi_remove, + .driver = { + .name = "xlnx-dsi", + .of_match_table = xlnx_dsi_of_match, + }, +}; + +module_platform_driver(dsi_driver); + +MODULE_AUTHOR("Venkateshwar Rao Gannavarapu venkateshwar.rao.gannavarapu@xilinx.com"); +MODULE_DESCRIPTION("Xilinx MIPI DSI host controller driver"); +MODULE_LICENSE("GPL"); -- 1.8.3.1
Hi GVRao,
Thank you for the patch.
On Thu, Jun 16, 2022 at 07:47:36PM +0530, Venkateshwar Rao Gannavarapu wrote:
You can drop DRM_PANEL_BRIDGE, the driver doesn't need it.
s/choose/Choose/
I'd add at least platform_device.h to avoid depending on indirect inclusion of headers.
Not needed.
It's not necessarily a panel. You can write
dev_err(dsi->dev, "Mismatch of lanes, host: %u != device: %u\n", dsi->lanes, device->lanes);
Same here.
As dev is the same as dsi->dev, I'd use dev->dsi here and drop the local variable.
I don't think the device_found flag mechanism is right. You remove the bridge in xlnx_dsi_host_detach(), so it should be added back here, shouldn't it ?
The next question would then be what to do with the next bridge acquired with devm_drm_of_get_bridge(). It looks like we have a lifetime management issue here, and I don't think it's fair to ask you to fix the bridge/panel core to get this driver merged, so I'm fine keeping this for now even if it's incorrect. I'd like feedback from others though.
This holds on a single line.
As the cases are mutually exclusive, i will always be equal to 1. You can drop the MAX_INPUT_SEL_FORMATS macro, the i variable, allocate a single entry for input_fmts, and set inputs_fmts[0].
Can this ever happen ?
This doesn't look right. Is it a leftover ? You should instead call drm_bridge_detach() on the next bridge.
static const, and you can move it as the first variable of the function.
Shouldn't this be done in the .atomic_enable() handler instead, possible through runtime PM ?
OK, you'll need to enable clocks for this too, so runtime PM could be a good option.
dsi->lanes is unsigned, so %u.
This leaves the clocks enabled.
This is read from a register, not parsed from DT, so the message isn't valid.
You could combine both messages into a single one.
Are you missing a return 0 here ?
dri-devel@lists.freedesktop.org
-
Laurent Pinchart -
Randy Dunlap -
Venkateshwar Rao Gannavarapu