Re: [DPU PATCH 1/2] drm/panel: Add Truly Dual DSI video mode panel

[Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

Hi Abhinav,

Thanks for posting this driver. Some comments below.

On Saturday 07 April 2018 12:36 PM, Abhinav Kumar wrote:
> From: Archit Taneja <architt@codeaurora.org>
> 
> Add support for truly dual DSI video mode panel
> panel used in MSM reference platforms >
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> ---
>   .../bindings/display/truly,dual_wqxga.txt          |  47 ++
>   drivers/gpu/drm/panel/Kconfig                      |   7 +
>   drivers/gpu/drm/panel/Makefile                     |   1 +
>   drivers/gpu/drm/panel/panel-truly-dual-dsi.c       | 530 +++++++++++++++++++++
>   4 files changed, 585 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/truly,dual_wqxga.txt
>   create mode 100644 drivers/gpu/drm/panel/panel-truly-dual-dsi.c
> 
> diff --git a/Documentation/devicetree/bindings/display/truly,dual_wqxga.txt b/Documentation/devicetree/bindings/display/truly,dual_wqxga.txt
> new file mode 100644
> index 0000000..a1b24c1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/truly,dual_wqxga.txt
> @@ -0,0 +1,47 @@
> +Truly model NT35597 1440x2560 DSI Panel
> +
> +Required properties:
> +- compatible: should be "truly,dual_wqxga"

The compatible string, kernel config and the driver file should be based
on the panel model no. There can be many truly based panels that
support wqxga. Something like "truly,nt35597" would be better.

> +- vdda-supply: phandle of the regulator that provides the supply voltage
> +  Power IC supply
> +- lab-supply: phandle of the regulator that provides the supply voltage
> +  for LCD bias
> +- ibb-supply: phandle of the regulator that provides the supply voltage
> +  for LCD bias

Both seem to have the same description. Aren't lab and ibb qualcomm
specific terms? Could we use the pin names specified in the panel's
data sheet?

> +- reset-gpios: phandle of gpio for reset line
> +  This should be 8mA, gpio can be configured using mux, pinctrl, pinctrl-names
> +- mode-gpios: phandle of the gpio for choosing the mode of the display
> +  for single DSI or Dual DSI

Could we describe here how to use this gpio? I.e, whether we need to set
it to low for dual DSI, etc?

> +- display-timings: Node for the Panel timings
> +- link2: phandle to the secondary node of the panel

The link2 binding was a temporary hack we used. We should use the
of-graph bindings to represent the two DSI input ports of the panel.

> +
> +Example:
> +
> +	dsi@ae94000 {
> +		panel@0 {
> +			compatible = "truly,dual_wqxga";
> +			reg = <0>;
> +			link2 = <&link2>;
> +			vdda-supply = <&pm8998_l14>;
> +
> +			pinctrl-names = "default", "suspend";
> +			pinctrl-0 = <&sde_dsi_active>;
> +			pinctrl-1 = <&sde_dsi_suspend>;
> +
> +			reset-gpios = <&tlmm 6 0>;
> +			mode-gpios = <&tlmm 52 0>;
> +			display-timings {
> +				timing0: timing-0 {
> +					clock-frequency = <268316138>;
> +					hactive = <1440>;
> +					vactive = <2560>;
> +					hfront-porch = <200>;
> +					hback-porch = <64>;
> +					hsync-len = <32>;
> +					vfront-porch = <8>;
> +					vback-porch = <7>;
> +					vsync-len = <1>;
> +				};
> +			};
> +		};
> +	};
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 988048e..a63c3f7 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -168,4 +168,11 @@ config DRM_PANEL_SITRONIX_ST7789V
>   	  Say Y here if you want to enable support for the Sitronix
>   	  ST7789V controller for 240x320 LCD panels
>   
> +config DRM_PANEL_TRULY_WQXGA
> +	tristate "Truly WQXGA"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	help
> +	  Say Y here if you want to enable support for Truly WQXGA Dual DSI
> +	  Video Mode panel
>   endmenu
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 3d2a88d..64891f6 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -17,3 +17,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
>   obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
>   obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
>   obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> +obj-$(CONFIG_DRM_PANEL_TRULY_WQXGA) += panel-truly-dual-dsi.o
> diff --git a/drivers/gpu/drm/panel/panel-truly-dual-dsi.c b/drivers/gpu/drm/panel/panel-truly-dual-dsi.c
> new file mode 100644
> index 0000000..47891ee
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-truly-dual-dsi.c
> @@ -0,0 +1,530 @@
> +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.

We should use the SPDX license headers here.

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/pinctrl/consumer.h>
> +
> +#include <video/mipi_display.h>
> +#include <video/of_videomode.h>
> +#include <video/videomode.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_mipi_dsi.h>
> +
> +struct truly_wqxga {
> +	struct device *dev;
> +	struct drm_panel panel;
> +
> +	struct regulator_bulk_data supplies[4];
> +
> +	struct gpio_desc *reset_gpio;
> +	struct gpio_desc *mode_gpio;
> +
> +	struct backlight_device *backlight;
> +	struct videomode vm;
> +
> +	struct mipi_dsi_device *dsi[2];
> +
> +	bool prepared;
> +	bool enabled;
> +};
> +
> +static inline struct truly_wqxga *panel_to_truly_wqxga(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct truly_wqxga, panel);
> +}
> +
> +static int truly_wqxga_power_on(struct truly_wqxga *ctx)
> +{
> +	int ret;
> +
> +	ret = regulator_set_load(ctx->supplies[1].consumer, 62000);
> +	if (ret)
> +		return ret;
> +
> +	ret = regulator_set_load(ctx->supplies[2].consumer, 100000);
> +	if (ret)
> +		return ret;
> +
> +	ret = regulator_set_load(ctx->supplies[3].consumer, 100000);
> +	if (ret)
> +		return ret;

We could make a const static struct array specifying the regulator
names (to be used during probe) and their corresponding power on and
power off loads. That should make things a bit cleaner.

> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	msleep(20);
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	usleep_range(20000, 20100);
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(20000, 20100);
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	usleep_range(50000, 50100);
> +
> +	/* dual port */
> +	gpiod_set_value(ctx->mode_gpio, 0);
> +
> +	return 0;
> +}
> +
> +static int truly_wqxga_power_off(struct truly_wqxga *ctx)
> +{
> +	int ret;
> +
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	ret = regulator_set_load(ctx->supplies[1].consumer, 62000);
> +	if (ret)
> +		return ret;
> +
> +	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +}
> +
> +static int truly_wqxga_disable(struct drm_panel *panel)
> +{
> +	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
> +
> +	if (!ctx->enabled)
> +		return 0;
> +
> +	if (ctx->backlight) {
> +		ctx->backlight->props.power = FB_BLANK_POWERDOWN;
> +		backlight_update_status(ctx->backlight);
> +	}
> +
> +	ctx->enabled = false;
> +	return 0;
> +}
> +
> +static int truly_wqxga_unprepare(struct drm_panel *panel)
> +{
> +	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
> +	struct mipi_dsi_device **dsis = ctx->dsi;
> +	int ret = 0, i;
> +
> +	if (!ctx->prepared)
> +		return 0;
> +
> +	dsis[0]->mode_flags = 0;
> +	dsis[1]->mode_flags = 0;
> +
> +	for (i = 0; i < 2; i++)
> +		if (mipi_dsi_dcs_set_display_off(dsis[i]) < 0)
> +			ret = -ECOMM;
> +	msleep(78);
> +
> +	for (i = 0; i < 2; i++)
> +		if (mipi_dsi_dcs_enter_sleep_mode(dsis[i]) < 0)
> +			ret = -ECOMM;
> +	msleep(78);
> +
> +	truly_wqxga_power_off(ctx);
> +
> +	ctx->prepared = false;
> +	return ret;
> +}
> +
> +#define MAX_LEN	5
> +struct {
> +	u8 commands[MAX_LEN];
> +	int size;
> +} panel_cmds[] = { /* CMD2_P0 */
> +		   { { 0xff, 0x20 }, 2 },
> +		   { { 0xfb, 0x01 }, 2 },
> +		   { { 0x00, 0x01 }, 2 },
> +		   { { 0x01, 0x55 }, 2 },
> +		   { { 0x02, 0x45 }, 2 },
> +		   { { 0x05, 0x40 }, 2 },
> +		   { { 0x06, 0x19 }, 2 },
> +		   { { 0x07, 0x1e }, 2 },
> +		   { { 0x0b, 0x73 }, 2 },
> +		   { { 0x0c, 0x73 }, 2 },
> +		   { { 0x0e, 0xb0 }, 2 },
> +		   { { 0x0f, 0xae }, 2 },
> +		   { { 0x11, 0xb8 }, 2 },
> +		   { { 0x13, 0x00 }, 2 },
> +		   { { 0x58, 0x80 }, 2 },
> +		   { { 0x59, 0x01 }, 2 },
> +		   { { 0x5a, 0x00 }, 2 },
> +		   { { 0x5b, 0x01 }, 2 },
> +		   { { 0x5c, 0x80 }, 2 },
> +		   { { 0x5d, 0x81 }, 2 },
> +		   { { 0x5e, 0x00 }, 2 },
> +		   { { 0x5f, 0x01 }, 2 },
> +		   { { 0x72, 0x11 }, 2 },
> +		   { { 0x68, 0x03 }, 2 },
> +		   /* CMD2_P4 */
> +		   { { 0xFF, 0x24 }, 2 },
> +		   { { 0xFB, 0x01 }, 2 },
> +		   { { 0x00, 0x1C }, 2 },
> +		   { { 0x01, 0x0B }, 2 },
> +		   { { 0x02, 0x0C }, 2 },
> +		   { { 0x03, 0x01 }, 2 },
> +		   { { 0x04, 0x0F }, 2 },
> +		   { { 0x05, 0x10 }, 2 },
> +		   { { 0x06, 0x10 }, 2 },
> +		   { { 0x07, 0x10 }, 2 },
> +		   { { 0x08, 0x89 }, 2 },
> +		   { { 0x09, 0x8A }, 2 },
> +		   { { 0x0A, 0x13 }, 2 },
> +		   { { 0x0B, 0x13 }, 2 },
> +		   { { 0x0C, 0x15 }, 2 },
> +		   { { 0x0D, 0x15 }, 2 },
> +		   { { 0x0E, 0x17 }, 2 },
> +		   { { 0x0F, 0x17 }, 2 },
> +		   { { 0x10, 0x1C }, 2 },
> +		   { { 0x11, 0x0B }, 2 },
> +		   { { 0x12, 0x0C }, 2 },
> +		   { { 0x13, 0x01 }, 2 },
> +		   { { 0x14, 0x0F }, 2 },
> +		   { { 0x15, 0x10 }, 2 },
> +		   { { 0x16, 0x10 }, 2 },
> +		   { { 0x17, 0x10 }, 2 },
> +		   { { 0x18, 0x89 }, 2 },
> +		   { { 0x19, 0x8A }, 2 },
> +		   { { 0x1A, 0x13 }, 2 },
> +		   { { 0x1B, 0x13 }, 2 },
> +		   { { 0x1C, 0x15 }, 2 },
> +		   { { 0x1D, 0x15 }, 2 },
> +		   { { 0x1E, 0x17 }, 2 },
> +		   { { 0x1F, 0x17 }, 2 },
> +		   /* STV */
> +		   { { 0x20, 0x40 }, 2 },
> +		   { { 0x21, 0x01 }, 2 },
> +		   { { 0x22, 0x00 }, 2 },
> +		   { { 0x23, 0x40 }, 2 },
> +		   { { 0x24, 0x40 }, 2 },
> +		   { { 0x25, 0x6D }, 2 },
> +		   { { 0x26, 0x40 }, 2 },
> +		   { { 0x27, 0x40 }, 2 },
> +		   /* Vend */
> +		   { { 0xE0, 0x00 }, 2 },
> +		   { { 0xDC, 0x21 }, 2 },
> +		   { { 0xDD, 0x22 }, 2 },
> +		   { { 0xDE, 0x07 }, 2 },
> +		   { { 0xDF, 0x07 }, 2 },
> +		   { { 0xE3, 0x6D }, 2 },
> +		   { { 0xE1, 0x07 }, 2 },
> +		   { { 0xE2, 0x07 }, 2 },
> +		   /* UD */
> +		   { { 0x29, 0xD8 }, 2 },
> +		   { { 0x2A, 0x2A }, 2 },
> +		   /* CLK */
> +		   { { 0x4B, 0x03 }, 2 },
> +		   { { 0x4C, 0x11 }, 2 },
> +		   { { 0x4D, 0x10 }, 2 },
> +		   { { 0x4E, 0x01 }, 2 },
> +		   { { 0x4F, 0x01 }, 2 },
> +		   { { 0x50, 0x10 }, 2 },
> +		   { { 0x51, 0x00 }, 2 },
> +		   { { 0x52, 0x80 }, 2 },
> +		   { { 0x53, 0x00 }, 2 },
> +		   { { 0x56, 0x00 }, 2 },
> +		   { { 0x54, 0x07 }, 2 },
> +		   { { 0x58, 0x07 }, 2 },
> +		   { { 0x55, 0x25 }, 2 },
> +		   /* Reset XDONB */
> +		   { { 0x5B, 0x43 }, 2 },
> +		   { { 0x5C, 0x00 }, 2 },
> +		   { { 0x5F, 0x73 }, 2 },
> +		   { { 0x60, 0x73 }, 2 },
> +		   { { 0x63, 0x22 }, 2 },
> +		   { { 0x64, 0x00 }, 2 },
> +		   { { 0x67, 0x08 }, 2 },
> +		   { { 0x68, 0x04 }, 2 },
> +		   /* Resolution:1440x2560 */
> +		   { { 0x72, 0x02 }, 2 },
> +		   /* mux */
> +		   { { 0x7A, 0x80 }, 2 },
> +		   { { 0x7B, 0x91 }, 2 },
> +		   { { 0x7C, 0xD8 }, 2 },
> +		   { { 0x7D, 0x60 }, 2 },
> +		   { { 0x7F, 0x15 }, 2 },
> +		   { { 0x75, 0x15 }, 2 },
> +		   /* ABOFF */
> +		   { { 0xB3, 0xC0 }, 2 },
> +		   { { 0xB4, 0x00 }, 2 },
> +		   { { 0xB5, 0x00 }, 2 },
> +		   /* Source EQ */
> +		   { { 0x78, 0x00 }, 2 },
> +		   { { 0x79, 0x00 }, 2 },
> +		   { { 0x80, 0x00 }, 2 },
> +		   { { 0x83, 0x00 }, 2 },
> +		   /* FP BP */
> +		   { { 0x93, 0x0A }, 2 },
> +		   { { 0x94, 0x0A }, 2 },
> +		   /* Inversion Type */
> +		   { { 0x8A, 0x00 }, 2 },
> +		   { { 0x9B, 0xFF }, 2 },
> +		   /* IMGSWAP =1 @PortSwap=1 */
> +		   { { 0x9D, 0xB0 }, 2 },
> +		   { { 0x9F, 0x63 }, 2 },
> +		   { { 0x98, 0x10 }, 2 },
> +		   /* FRM */
> +		   { { 0xEC, 0x00 }, 2 },
> +		   /* CMD1 */
> +		   { { 0xFF, 0x10 }, 2 },
> +		    /* VBP+VSA=,VFP = 10H */
> +		   { { 0x3B, 0x03, 0x0A, 0x0A, }, 4 },
> +		   /* FTE on */
> +		   { { 0x35, 0x00 }, 2 },
> +		   /* EN_BK =1(auto black) */
> +		   { { 0xE5, 0x01 }, 2 },
> +		   /* CMD mode(10) VDO mode(03) */
> +		   { { 0xBB, 0x03 }, 2 },
> +		   /* Non Reload MTP */
> +		   { { 0xFB, 0x01 }, 2 },
> +		   /* SlpOut + DispOn */
> +		   //05 01 00 00 78 00 02 11 00
> +		   //05 01 00 00 78 00 02 29 00

We can drop the commented lines. If possible, it would be nice
to have some more description about the registers, but I can imagine
that info being hard to retrieve.

> +};
> +
> +static int truly_wqxga_prepare(struct drm_panel *panel)
> +{
> +	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
> +	struct mipi_dsi_device **dsis = ctx->dsi;
> +	struct mipi_dsi_device *d;
> +	int ret, i, j;
> +
> +	if (ctx->prepared)
> +		return 0;
> +
> +	ret = truly_wqxga_power_on(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsis[0]->mode_flags |= MIPI_DSI_MODE_LPM;
> +	dsis[1]->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	for (j = 0; j < ARRAY_SIZE(panel_cmds); j++) {
> +		for (i = 0; i < 2; i++) {
> +			d = dsis[i];
> +			ret = mipi_dsi_dcs_write_buffer(dsis[i],
> +					panel_cmds[j].commands,
> +					panel_cmds[j].size);
> +			if (ret < 0) {
> +				/* continue anyway */
> +				dev_err(ctx->dev, "failed to cmd no %d, err: %d\n",
> +						j, ret);

This may have been done for debug purposes. It would be probably better
to bail out.

> +			}
> +		}
> +	}
> +
> +
> +	for (i = 0; i < 2; i++)
> +		if (mipi_dsi_dcs_exit_sleep_mode(dsis[i]) < 0) {
> +			dev_err(ctx->dev, "failed to exit sleep mode\n");
> +			return -ECOMM;
> +		}
> +	msleep(78);
> +
> +	for (i = 0; i < 2; i++)
> +		if (mipi_dsi_dcs_set_display_on(dsis[i]) < 0) {
> +			dev_err(ctx->dev, "failed to send display on\n");
> +			return -ECOMM;
> +		}
> +	msleep(78);
> +
> +	ctx->prepared = true;
> +
> +	return 0;
> +}
> +
> +static int truly_wqxga_enable(struct drm_panel *panel)
> +{
> +	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
> +
> +	if (ctx->enabled)
> +		return 0;
> +
> +	if (ctx->backlight) {
> +		ctx->backlight->props.power = FB_BLANK_UNBLANK;
> +		backlight_update_status(ctx->backlight);
> +	}
> +	ctx->enabled = true;
> +
> +	return 0;
> +}
> +
> +static int truly_wqxga_get_modes(struct drm_panel *panel)
> +{
> +	struct drm_connector *connector = panel->connector;
> +	struct truly_wqxga *ctx = panel_to_truly_wqxga(panel);
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_create(connector->dev);
> +	if (!mode) {
> +		dev_err(ctx->dev, "failed to create a new display mode\n");
> +		return 0;
> +	}
> +
> +	drm_display_mode_from_videomode(&ctx->vm, mode);
> +	connector->display_info.width_mm = 74;
> +	connector->display_info.height_mm = 131;
> +	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1;
> +}
> +
> +static const struct drm_panel_funcs truly_wqxga_drm_funcs = {
> +	.disable = truly_wqxga_disable,
> +	.unprepare = truly_wqxga_unprepare,
> +	.prepare = truly_wqxga_prepare,
> +	.enable = truly_wqxga_enable,
> +	.get_modes = truly_wqxga_get_modes,
> +};
> +
> +static int truly_wqxga_panel_add(struct truly_wqxga *ctx)
> +{
> +	struct device *dev = ctx->dev;
> +	int ret;
> +
> +	ctx->supplies[0].supply = "vddio";
> +	ctx->supplies[1].supply = "vdda";
> +	ctx->supplies[2].supply = "lab";
> +	ctx->supplies[3].supply = "ibb";
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->reset_gpio)) {
> +		dev_err(dev, "cannot get reset-gpios %ld\n",
> +			PTR_ERR(ctx->reset_gpio));
> +		return PTR_ERR(ctx->reset_gpio);
> +	}
> +
> +	ctx->mode_gpio = devm_gpiod_get(dev, "mode", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->mode_gpio)) {
> +		dev_err(dev, "cannot get mode gpio %ld\n",
> +			PTR_ERR(ctx->mode_gpio));
> +		ctx->mode_gpio = NULL;
> +		return PTR_ERR(ctx->mode_gpio);
> +	}
> +
> +	ret = pinctrl_pm_select_default_state(dev);
> +	if (ret) {
> +		dev_err(dev, "%s: failed to set pinctrl default state, %d\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	ret = of_get_videomode(dev->of_node, &ctx->vm, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	drm_panel_init(&ctx->panel);
> +	ctx->panel.dev = dev;
> +	ctx->panel.funcs = &truly_wqxga_drm_funcs;
> +	drm_panel_add(&ctx->panel);
> +
> +	return 0;
> +}
> +
> +static void truly_wqxga_panel_del(struct truly_wqxga *ctx)
> +{
> +	if (ctx->panel.dev)
> +		drm_panel_remove(&ctx->panel);
> +
> +	if (ctx->backlight)
> +		put_device(&ctx->backlight->dev);
> +
> +	if (ctx->dsi[1])
> +		put_device(&ctx->dsi[1]->dev);
> +}
> +
> +static int truly_wqxga_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct truly_wqxga *ctx;
> +	struct mipi_dsi_device *secondary = NULL;
> +	struct device_node *np;
> +	int ret;
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_CLOCK_NON_CONTINUOUS |
> +			  MIPI_DSI_MODE_LPM;
> +
> +	/* Only DSI-LINK1 node has "link2" entry. */
> +	np = of_parse_phandle(dsi->dev.of_node, "link2", 0);
> +	if (np) {
> +		secondary = of_find_mipi_dsi_device_by_node(np);
> +		of_node_put(np);
> +
> +		if (!secondary)
> +			return -EPROBE_DEFER;
> +
> +		ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +		if (!ctx) {
> +			put_device(&secondary->dev);
> +			return -ENOMEM;
> +		}
> +		mipi_dsi_set_drvdata(dsi, ctx);
> +
> +		ctx->dev = dev;
> +		ctx->dsi[0] = dsi;
> +		ctx->dsi[1] = secondary;
> +
> +		ret = truly_wqxga_panel_add(ctx);
> +		if (ret) {
> +			put_device(&secondary->dev);
> +			return ret;
> +		}
> +	}

This should go when we use the of-graph bindings. The second
mipi_dsi_device needs to be created manually in the driver
using mipi_dsi_device_register_full(). You could look at the
ADV7511 bridge driver as an example. We would eventually need
to call mipi_dsi_attach() on the second device too.

Thanks,
Archit

> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		if (secondary)
> +			truly_wqxga_panel_del(ctx);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int truly_wqxga_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct truly_wqxga *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	mipi_dsi_detach(dsi);
> +
> +	/* delete panel only for the DSI1 interface */
> +	if (ctx)
> +		truly_wqxga_panel_del(ctx);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id truly_wqxga_of_match[] = {
> +	{ .compatible = "truly,dual_wqxga", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, truly_wqxga_of_match);
> +
> +static struct mipi_dsi_driver truly_wqxga_driver = {
> +	.driver = {
> +		.name = "panel_truly_wqxga",
> +		.of_match_table = truly_wqxga_of_match,
> +	},
> +	.probe = truly_wqxga_probe,
> +	.remove = truly_wqxga_remove,
> +};
> +module_mipi_dsi_driver(truly_wqxga_driver);
> +
> +MODULE_DESCRIPTION("truly wqxga DSI Panel Driver");
> +MODULE_LICENSE("GPL v2");
> 
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno