aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Mon May 17, 2021 8:57 am

milo246 wrote:
Mon May 17, 2021 8:44 am
aBUGSworstnightmare wrote:
Mon May 17, 2021 8:26 am
we can provide LVDS timing as min/typ/max so the routines should stay within that boundries, hence display will be operated in spec still.
In theory yes. But in practice only the "typical" (center) values appear to get passed around. It's difficult to automate as the timing requirements may be anything, ranging from "hsync is ignored" to "frontporch must be even".

My method was to open a Python commandline and muck about with the numbers until it's close to 60Hz and everything close to typical.
yes, that' fully valid. So this 'task' has to be made when adding the panel timings.
But then it might be more simple to add them to the DT overlay and not to 'panel-simple.c' or 'panel-lvds.c'as later two would ask for a kernel compile and having them in DT is compiling the overlay only.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 11246
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Rpi 4 with DRM and 7inch panel using kms driver

Mon May 17, 2021 12:06 pm

Sorry, tied up with lots of other stuff.
UberDoefus wrote:
Mon May 17, 2021 6:38 am
Will post the complete driver here icw the DTS. I'm using the Marek driver but with some changes, will need to figure out which version I'm using. I'm new to providing stuff to the upstream kernel, so do I make my changes known to Marek or do I create a new pull-request?
Upstream do their own thing. You can follow the full thread at https://patchwork.freedesktop.org/series/89914/
They're testing on other hardware and have it working.

vc4_dsi is doing the wrong thing in not supporting the mode_valid/mode_set/mode_fixup hooks.
It needs to split the chain of devices in two as pre_enable calls backwards from the connector to the source, and bridges/panels are entitled to send DSI commands from pre_enable. Without the split the PHY isn't powered up, so it'll fail.
We can't do the same as in pre_enable/enable where we call the bridges/panels directly as we're missing some of the state required.

I'm not clear how other platforms handle this, but I think the simplest answer is to power the PHY up in bind, and leave it up. This only happens if there is a legitimate panel/bridge attached, so there's no real power saving, and it allows the commands to be sent. I'll be investigating whether this is feasible as an overall solution.

Mainline Linux is still based on mailing lists. You can join in the threads on dri-devel if you wish, but currently there is little really to report without fixing up vc4_dsi.
UberDoefus wrote:Btw, when looking into the vc4_dsi.c into the lane configuration, I see something odd:

Code: Select all

# define DSI1_PHY_AFEC0_IDR_DLANE3_MASK		VC4_MASK(31, 29)
# define DSI1_PHY_AFEC0_IDR_DLANE3_SHIFT	29
# define DSI1_PHY_AFEC0_IDR_DLANE2_MASK		VC4_MASK(28, 26)
# define DSI1_PHY_AFEC0_IDR_DLANE2_SHIFT	26
# define DSI1_PHY_AFEC0_IDR_DLANE1_MASK		VC4_MASK(27, 23)
# define DSI1_PHY_AFEC0_IDR_DLANE1_SHIFT	23
# define DSI1_PHY_AFEC0_IDR_DLANE0_MASK		VC4_MASK(22, 20)
# define DSI1_PHY_AFEC0_IDR_DLANE0_SHIFT	20
# define DSI1_PHY_AFEC0_IDR_CLANE_MASK		VC4_MASK(19, 17)
# define DSI1_PHY_AFEC0_IDR_CLANE_SHIFT		17
# define DSI0_PHY_AFEC0_ACTRL_DLANE1_MASK	VC4_MASK(23, 20)
# define DSI0_PHY_AFEC0_ACTRL_DLANE1_SHIFT	20
# define DSI0_PHY_AFEC0_ACTRL_DLANE0_MASK	VC4_MASK(19, 16)
# define DSI0_PHY_AFEC0_ACTRL_DLANE0_SHIFT	16
# define DSI0_PHY_AFEC0_ACTRL_CLANE_MASK	VC4_MASK(15, 12)
# define DSI0_PHY_AFEC0_ACTRL_CLANE_SHIFT	12
dsi1 dlane3 is 3 bits wide, shifted 29 positions -> ok
dsi1 dlane2 is 3 bits wide, shifted 26 positions -> ok
dsi1 dlane1 is 5 bits wide, shifted 23 positions -> nok, this is an overlap with dlane2
dsi1 dlane0 is 3 bits wide, shifted 20 positions -> ok
Yup, you're right, however it makes no functional difference as the only use is in

Code: Select all

		u32 afec0 = (VC4_SET_FIELD(7, DSI_PHY_AFEC0_PTATADJ) |
			     VC4_SET_FIELD(7, DSI_PHY_AFEC0_CTATADJ) |
			     VC4_SET_FIELD(6, DSI1_PHY_AFEC0_IDR_CLANE) |
			     VC4_SET_FIELD(6, DSI1_PHY_AFEC0_IDR_DLANE0) |
			     VC4_SET_FIELD(6, DSI1_PHY_AFEC0_IDR_DLANE1) |
			     VC4_SET_FIELD(6, DSI1_PHY_AFEC0_IDR_DLANE2) |
			     VC4_SET_FIELD(6, DSI1_PHY_AFEC0_IDR_DLANE3));
which never clears the bits based on the mask. (It masks the value of 6 with the mask, before then shifting it by the appropriate number of bits.
milo246 wrote:
Mon May 17, 2021 7:54 am
About the DSI clock, the RPi has a "constant" parent clock that it doesn't want to change. On the RPi 4 this runs at 3000 MHz. The DSI clock must be an integer divider of that, so you can get a 300MHz (/10) clock or a 500MHz (/6) but you cannot get a 425MHz clock. This will be rounded to 3000/7=428.57 MHz.

The vc4_dsi driver will bluntly adjust the horizontal timing to compensate, without regard for panel capability, so this may go completely out of spec.

What I do to get around that is to make the LVDS clock something that is attainable and within range. So instead of aiming at the "typical" value in the datasheet I go for something that's actually possible, so that the vc4_dsi does not adjust the timing.

For example, a 50 MHz LVDS clock, using a /6 divider so the DSI runs at 300MHz. Then adjust the horizontal/vertical timings to get about 60Hz refresh.
The mode will be adjusted for the INPUT to the bridge. The bridge driver should have the original panel configuration for setting the OUTPUT of the bridge.
This is how DSI burst mode (which is supported by SN65DSI8x) works - the DSI link is run at a significantly faster speed than the panel requires, and drops to a low power state between these bursts.
It's being tripped up by the split bridge/panel config, and the mode_fixup hook not being passed through.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

milo246
Posts: 73
Joined: Mon Mar 01, 2021 1:43 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Mon May 17, 2021 12:23 pm

Just compiled the "marek" driver and adapted my devicetree so port 2 is the output.

Driver silently doesn't load. No error, nothing. Bleh. I'll add some printks first to figure out what's wrong.

(edit) Oh, the CONFIG name is different
# CONFIG_DRM_TI_SN65DSI83 is not set

UberDoefus
Posts: 19
Joined: Tue May 04, 2021 8:28 am

Re: Rpi 4 with DRM and 7inch panel using kms driver

Mon May 17, 2021 1:07 pm

As promised, my used DTS and the modified Marek bridge driver V4 which allows me to use the fb-test and cat random to fb0:

DTS:

Code: Select all

/*
 * vc4-kms-dsi-sn65dsi8x-overlay.dts
 */

/dts-v1/;
/plugin/;
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/pinctrl/bcm2835.h>
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/clock/bcm2835.h>

#include "cma-overlay.dts"

/ {
	compatible = "brcm,bcm2835";

	/* PWM0 function */
	fragment@0 {
		target = <&gpio>;
		__overlay__ {
			pwm_pins: pwm_pins {
				brcm,pins = <18>; /* pin 49 */
				brcm,function = <BCM2835_FSEL_ALT5>;
			};

			dsi_lvds_en: dsi_lvds_en {
				brcm,pins = <11>;
				brcm,function = <BCM2835_FSEL_GPIO_OUT>;
			};

			i2c1_pins: i2c1 {
				brcm,pins = <44 45>;
				brcm,function = <BCM2835_FSEL_ALT2>;
				brcm,pull = <BCM2835_PUD_UP>;
			};

			backlight_en: backlight_en {
				brcm,pins = <17>; /* pin 50 */
				brcm,function = <BCM2835_FSEL_GPIO_OUT>;
			};
		};
	};

	fragment@1 {
		target = <&pwm>;
		frag1: __overlay__ {
			pinctrl-names = "default";
			pinctrl-0 = <&pwm_pins>;
			assigned-clock-rates = <100000000>;
			status = "okay";
		};
	};

	fragment@2 {
		target-path = "/";
		__overlay__ {
			backlight_lvds: backlight {
				compatible = "pwm-backlight";
				pwms = <&pwm 0 5000000>; /* 200Hz */
				brightness-levels = <0 1000>;
				num-interpolated-steps = <1000>;
				default-brightness-level = <800>;
				enable-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;

				power-supply = <&vdd_3v3_reg>;
				status = "okay";
			};

			panel: panel {
				compatible = "innolux,g156hce-l01", "panel-lvds";
				label = "TIM: fancom display";
				status = "okay";

				backlight = <&backlight_lvds>;
				power-supply = <&vdd_3v3_reg>;

				width-mm = <344>;
				height-mm = <194>;

				data-mapping = "vesa-24";

				panel-timing {
					/* clock frequency of 70.93Mhz in the datasheet is for single LVDS channel */
					/* since we use dual LVDS channel, the clock frequency is 2*70.93Mhz */
					/* the bridge driver will correct this */
					/* vc4_dsi.c: however, the dsi parent_rate is 3Ghz and the dsi-divider is fixed to (24/#nr-data lanes) */
					/* (3Ghz / 3)/7 = 142.86Mhz. (142.86Mhz * 24bpp) / 8 = 571Mhz DSI clock */
					/* (3Ghz / 4)/7 = 107.14Mhz. (107.14Mhz * 24bpp) / 8 = 428.57Mhz DSI clock */
					/* minimum required DSI clock is (48bits/(3 lanes * 2)) * 60Mhz = 480Mhz
					/* we'll hence need a 571Mhz capped to 500Mhz DSI clock, with a divider of 8 in the bridge, resulting in 62.5Mhz */
					/* 62.5Mhz * 2 = 125Mhz */
					clock-frequency = <125000000>;

					/* the hactive and porches in the datasheet are for a single LVDS channel */
					/* since we use dual LVDS channel, double everything */
					hactive = <1920>;
					hsync-len = <60>;
					hfront-porch = <60>;
					hback-porch = <60>;

					vactive = <1080>;
					vsync-len = <4>;
					vfront-porch = <3>;
					vback-porch = <3>;

					hsync-active = <0>;
					vsync-active = <0>;
					de-active = <1>;
					pixelclk-active = <1>;
				};

				ports {
					port@0 {
						reg = <0>;
						dual-lvds-odd-pixels;
						panel_in_lvds_channelA: endpoint {
							remote-endpoint = <&bridge_out_channelA>;
						};
					};

					port@1 {
						reg = <1>;
						dual-lvds-even-pixels;
						panel_in_lvds_channelB: endpoint {
							remote-endpoint = <&bridge_out_channelB>;
						};
					};
				};
			};
		};
	};

	fragment@3 {
		target = <&i2c1>;
		__overlay__ {
			#gpio-cells = <2>;
			#address-cells = <1>;
			#size-cells = <0>;
			status = "okay";

			pinctrl-names = "default";
			pinctrl-0 = <&i2c1_pins>;
			clock-frequency = <400000>;

			sn65dsi84@2c {
				compatible = "ti,sn65dsi84";
				reg = <0x2c>;
				enable-gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;

				pinctrl-names = "default";
				pinctrl-0 = <&dsi_lvds_en>;
				status = "okay";

				interrupts = <36 IRQ_TYPE_LEVEL_HIGH>;

				ports {
					#address-cells = <1>;
					#size-cells = <0>;

					port@0 {
						reg = <0>;
						bridge_in: endpoint {
							remote-endpoint = <&dsi_out_port>;
							data-lanes = <0 1 2>;
						};
					};

					port@2 {
						reg = <2>;
						bridge_out_channelA: endpoint {
							remote-endpoint = <&panel_in_lvds_channelA>;
						};
					};

					port@3 {
						reg = <3>;
						bridge_out_channelB: endpoint {
							remote-endpoint = <&panel_in_lvds_channelB>;
						};
					};
				};
			};
		};
	};

	fragment@4 {
		target = <&dsi1>;
		__overlay__ {
			#address-cells = <1>;
			#size-cells = <0>;
			status = "okay";
			port {
				dsi_out_port: endpoint {
					remote-endpoint = <&bridge_in>;
					data-lanes = <0 1 2>;
				};
			};
		};
	};

	fragment@5 {
		target = <&i2c0if>;
		__overlay__ {
			status = "okay";
		};
	};
		
	fragment@6 {
		target = <&i2c0mux>;
		__overlay__ {
			status = "okay";
		};
	};

	fragment@20 {
		target = <&vc4>;
		__overlay__ {
			status = "okay";
		};
	};
	fragment@21 {
		target = <&hvs>;
		__overlay__ {
			status = "okay";
		};
	};
	fragment@22 {
		target = <&v3d>;
		__overlay__ {
			status = "okay";
		};
	};
	fragment@23 {
		target = <&clocks>;
		__overlay__ {
			claim-clocks = <
				BCM2835_PLLD_DSI0
				BCM2835_PLLD_DSI1
				BCM2835_PLLH_AUX
				BCM2835_PLLH_PIX
			>;
		};
	};

	fragment@24 {
		target = <&vec>;
		__overlay__ {
			status = "okay";
		};
	};

	fragment@25 {
		target = <&txp>;
		__overlay__ {
			status = "okay";
		};
	};

	fragment@26 {
		target = <&pixelvalve0>;
		__overlay__  {
			status = "okay";
		};
	};

	fragment@27 {
		target = <&pixelvalve1>;
		__overlay__  {
			status = "okay";
		};
	};

	fragment@28 {
		target = <&pixelvalve2>;
		__overlay__  {
			status = "okay";
		};
	};

	fragment@29 {
		target = <&pixelvalve3>;
		__overlay__  {
			status = "okay";
		};
	};

	fragment@30 {
		target = <&pixelvalve4>;
		__overlay__  {
			status = "okay";
		};
	};

	fragment@31 {
		target = <&fb>;
		__overlay__  {
			status = "okay";
		};
	};
};
Driver:

Code: Select all

#define MODE_HACK
//#define HARDCODED_REGS
//#define SN65DSI83_TEST_PATTERN

// SPDX-License-Identifier: GPL-2.0
/*
 * TI SN65DSI83,84,85 driver
 *
 * Currently supported:
 * - SN65DSI83
 *   = 1x Single-link DSI ~ 1x Single-link LVDS
 *   - Supported
 *   - Single-link LVDS mode tested
 * - SN65DSI84
 *   = 1x Single-link DSI ~ 2x Single-link or 1x Dual-link LVDS
 *   - Supported
 *   - Dual-link LVDS mode tested
 *   - 2x Single-link LVDS mode unsupported
 *     (should be easy to add by someone who has the HW)
 * - SN65DSI85
 *   = 2x Single-link or 1x Dual-link DSI ~ 2x Single-link or 1x Dual-link LVDS
 *   - Unsupported
 *     (should be easy to add by someone who has the HW)
 *
 * Copyright (C) 2021 Marek Vasut <marex@denx.de>
 *
 * Based on previous work of:
 * Valentin Raevsky <valentin@compulab.co.il>
 * Philippe Schenker <philippe.schenker@toradex.com>
 */

#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/of_graph.h>
#include <linux/regmap.h>

#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge.h>
#include <drm/drm_mipi_dsi.h>
#include <drm/drm_of.h>
#include <drm/drm_panel.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>

/* ID registers */
#define REG_ID(n)				(0x00 + (n))
/* Reset and clock registers */
#define REG_RC_RESET				0x09
#define  REG_RC_RESET_SOFT_RESET		BIT(0)
#define REG_RC_LVDS_PLL				0x0a
#define  REG_RC_LVDS_PLL_PLL_EN_STAT		BIT(7)
#define  REG_RC_LVDS_PLL_LVDS_CLK_RANGE(n)	(((n) & 0x7) << 1)
#define  REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY	BIT(0)
#define REG_RC_DSI_CLK				0x0b
#define  REG_RC_DSI_CLK_DSI_CLK_DIVIDER(n)	(((n) & 0x1f) << 3)
#define  REG_RC_DSI_CLK_REFCLK_MULTIPLIER(n)	((n) & 0x3)
#define REG_RC_PLL_EN				0x0d
#define  REG_RC_PLL_EN_PLL_EN			BIT(0)
/* DSI registers */
#define REG_DSI_LANE				0x10
#define  REG_DSI_LANE_LEFT_RIGHT_PIXELS		BIT(7)	/* DSI85-only */
#define  REG_DSI_LANE_DSI_CHANNEL_MODE_DUAL	0	/* DSI85-only */
#define  REG_DSI_LANE_DSI_CHANNEL_MODE_2SINGLE	BIT(6)	/* DSI85-only */
#define  REG_DSI_LANE_DSI_CHANNEL_MODE_SINGLE	BIT(5)
#define  REG_DSI_LANE_CHA_DSI_LANES(n)		(((n) & 0x3) << 3)
#define  REG_DSI_LANE_CHB_DSI_LANES(n)		(((n) & 0x3) << 1)
#define  REG_DSI_LANE_SOT_ERR_TOL_DIS		BIT(0)
#define REG_DSI_EQ				0x11
#define  REG_DSI_EQ_CHA_DSI_DATA_EQ(n)		(((n) & 0x3) << 6)
#define  REG_DSI_EQ_CHA_DSI_CLK_EQ(n)		(((n) & 0x3) << 2)
#define REG_DSI_CLK				0x12
#define  REG_DSI_CLK_CHA_DSI_CLK_RANGE(n)	((n) & 0xff)
/* LVDS registers */
#define REG_LVDS_FMT				0x18
#define  REG_LVDS_FMT_DE_NEG_POLARITY		BIT(7)
#define  REG_LVDS_FMT_HS_NEG_POLARITY		BIT(6)
#define  REG_LVDS_FMT_VS_NEG_POLARITY		BIT(5)
#define  REG_LVDS_FMT_LVDS_LINK_CFG		BIT(4)	/* 0:AB 1:A-only */
#define  REG_LVDS_FMT_CHA_24BPP_MODE		BIT(3)
#define  REG_LVDS_FMT_CHB_24BPP_MODE		BIT(2)
#define  REG_LVDS_FMT_CHA_24BPP_FORMAT1		BIT(1)
#define  REG_LVDS_FMT_CHB_24BPP_FORMAT1		BIT(0)
#define REG_LVDS_VCOM				0x19
#define  REG_LVDS_VCOM_CHA_LVDS_VOCM		BIT(6)
#define  REG_LVDS_VCOM_CHB_LVDS_VOCM		BIT(4)
#define  REG_LVDS_VCOM_CHA_LVDS_VOD_SWING(n)	(((n) & 0x3) << 2)
#define  REG_LVDS_VCOM_CHB_LVDS_VOD_SWING(n)	((n) & 0x3)
#define REG_LVDS_LANE				0x1a
#define  REG_LVDS_LANE_EVEN_ODD_SWAP		BIT(6)
#define  REG_LVDS_LANE_CHA_REVERSE_LVDS		BIT(5)
#define  REG_LVDS_LANE_CHB_REVERSE_LVDS		BIT(4)
#define  REG_LVDS_LANE_CHA_LVDS_TERM		BIT(1)
#define  REG_LVDS_LANE_CHB_LVDS_TERM		BIT(0)
#define REG_LVDS_CM				0x1b
#define  REG_LVDS_CM_CHA_LVDS_CM_ADJUST(n)	(((n) & 0x3) << 4)
#define  REG_LVDS_CM_CHB_LVDS_CM_ADJUST(n)	((n) & 0x3)
/* Video registers */
#define REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW	0x20
#define REG_VID_CHA_ACTIVE_LINE_LENGTH_HIGH	0x21
#define REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW	0x24
#define REG_VID_CHA_VERTICAL_DISPLAY_SIZE_HIGH	0x25
#define REG_VID_CHA_SYNC_DELAY_LOW		0x28
#define REG_VID_CHA_SYNC_DELAY_HIGH		0x29
#define REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW	0x2c
#define REG_VID_CHA_HSYNC_PULSE_WIDTH_HIGH	0x2d
#define REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW	0x30
#define REG_VID_CHA_VSYNC_PULSE_WIDTH_HIGH	0x31
#define REG_VID_CHA_HORIZONTAL_BACK_PORCH	0x34
#define REG_VID_CHA_VERTICAL_BACK_PORCH		0x36
#define REG_VID_CHA_HORIZONTAL_FRONT_PORCH	0x38
#define REG_VID_CHA_VERTICAL_FRONT_PORCH	0x3a
#define REG_VID_CHA_TEST_PATTERN		0x3c
/* sn65dsi85 only */
#define REG_VID_RIGHT_CROP              0x3d
#define REG_VID_LEFT_CROP               0x3e

/* IRQ registers */
#define REG_IRQ_GLOBAL				0xe0
#define  REG_IRQ_GLOBAL_IRQ_EN			BIT(0)
#define REG_IRQ_EN				0xe1
#define  REG_IRQ_EN_CHA_SYNCH_ERR_EN		BIT(7)
#define  REG_IRQ_EN_CHA_CRC_ERR_EN		BIT(6)
#define  REG_IRQ_EN_CHA_UNC_ECC_ERR_EN		BIT(5)
#define  REG_IRQ_EN_CHA_COR_ECC_ERR_EN		BIT(4)
#define  REG_IRQ_EN_CHA_LLP_ERR_EN		BIT(3)
#define  REG_IRQ_EN_CHA_SOT_BIT_ERR_EN		BIT(2)
#define  REG_IRQ_EN_CHA_PLL_UNLOCK_EN		BIT(0)
#define REG_IRQ_STAT				0xe5
#define  REG_IRQ_STAT_CHA_SYNCH_ERR		BIT(7)
#define  REG_IRQ_STAT_CHA_CRC_ERR		BIT(6)
#define  REG_IRQ_STAT_CHA_UNC_ECC_ERR		BIT(5)
#define  REG_IRQ_STAT_CHA_COR_ECC_ERR		BIT(4)
#define  REG_IRQ_STAT_CHA_LLP_ERR		BIT(3)
#define  REG_IRQ_STAT_CHA_SOT_BIT_ERR		BIT(2)
#define  REG_IRQ_STAT_CHA_PLL_UNLOCK		BIT(0)

enum sn65dsi83_model {
	MODEL_SN65DSI83,
	MODEL_SN65DSI84,
};

struct sn65dsi83 {
	struct drm_bridge		bridge;
	struct drm_display_mode		mode;
	struct device			*dev;
	struct regmap			*regmap;
	struct device_node		*host_node;
	struct mipi_dsi_device		*dsi;
	struct drm_bridge		*panel_bridge;
	struct gpio_desc		*enable_gpio;
	int				dsi_lanes;
	bool				lvds_dual_link;
	bool				lvds_dual_link_even_odd_swap;
	bool				lvds_format_24bpp;
	bool				lvds_format_jeida;
};

static const struct regmap_range sn65dsi83_readable_ranges[] = {
	regmap_reg_range(REG_ID(0), REG_ID(8)),
	regmap_reg_range(REG_RC_LVDS_PLL, REG_RC_DSI_CLK),
	regmap_reg_range(REG_RC_PLL_EN, REG_RC_PLL_EN),
	regmap_reg_range(REG_DSI_LANE, REG_DSI_CLK),
	regmap_reg_range(REG_LVDS_FMT, REG_LVDS_CM),
	regmap_reg_range(REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
			 REG_VID_CHA_ACTIVE_LINE_LENGTH_HIGH),
	regmap_reg_range(REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW,
			 REG_VID_CHA_VERTICAL_DISPLAY_SIZE_HIGH),
	regmap_reg_range(REG_VID_CHA_SYNC_DELAY_LOW,
			 REG_VID_CHA_SYNC_DELAY_HIGH),
	regmap_reg_range(REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
			 REG_VID_CHA_HSYNC_PULSE_WIDTH_HIGH),
	regmap_reg_range(REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
			 REG_VID_CHA_VSYNC_PULSE_WIDTH_HIGH),
	regmap_reg_range(REG_VID_CHA_HORIZONTAL_BACK_PORCH,
			 REG_VID_CHA_HORIZONTAL_BACK_PORCH),
	regmap_reg_range(REG_VID_CHA_VERTICAL_BACK_PORCH,
			 REG_VID_CHA_VERTICAL_BACK_PORCH),
	regmap_reg_range(REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
			 REG_VID_CHA_HORIZONTAL_FRONT_PORCH),
	regmap_reg_range(REG_VID_CHA_VERTICAL_FRONT_PORCH,
			 REG_VID_CHA_VERTICAL_FRONT_PORCH),
	regmap_reg_range(REG_VID_CHA_TEST_PATTERN, REG_VID_CHA_TEST_PATTERN),
	regmap_reg_range(REG_IRQ_GLOBAL, REG_IRQ_EN),
	regmap_reg_range(REG_IRQ_STAT, REG_IRQ_STAT),
};

static const struct regmap_access_table sn65dsi83_readable_table = {
	.yes_ranges = sn65dsi83_readable_ranges,
	.n_yes_ranges = ARRAY_SIZE(sn65dsi83_readable_ranges),
};

static const struct regmap_range sn65dsi83_writeable_ranges[] = {
	regmap_reg_range(REG_RC_RESET, REG_RC_DSI_CLK),
	regmap_reg_range(REG_RC_PLL_EN, REG_RC_PLL_EN),
	regmap_reg_range(REG_DSI_LANE, REG_DSI_CLK),
	regmap_reg_range(REG_LVDS_FMT, REG_LVDS_CM),
	regmap_reg_range(REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
			 REG_VID_CHA_ACTIVE_LINE_LENGTH_HIGH),
	regmap_reg_range(REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW,
			 REG_VID_CHA_VERTICAL_DISPLAY_SIZE_HIGH),
	regmap_reg_range(REG_VID_CHA_SYNC_DELAY_LOW,
			 REG_VID_CHA_SYNC_DELAY_HIGH),
	regmap_reg_range(REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
			 REG_VID_CHA_HSYNC_PULSE_WIDTH_HIGH),
	regmap_reg_range(REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
			 REG_VID_CHA_VSYNC_PULSE_WIDTH_HIGH),
	regmap_reg_range(REG_VID_CHA_HORIZONTAL_BACK_PORCH,
			 REG_VID_CHA_HORIZONTAL_BACK_PORCH),
	regmap_reg_range(REG_VID_CHA_VERTICAL_BACK_PORCH,
			 REG_VID_CHA_VERTICAL_BACK_PORCH),
	regmap_reg_range(REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
			 REG_VID_CHA_HORIZONTAL_FRONT_PORCH),
	regmap_reg_range(REG_VID_CHA_VERTICAL_FRONT_PORCH,
			 REG_VID_CHA_VERTICAL_FRONT_PORCH),
	regmap_reg_range(REG_VID_CHA_TEST_PATTERN, REG_VID_CHA_TEST_PATTERN),
	regmap_reg_range(REG_IRQ_GLOBAL, REG_IRQ_EN),
	regmap_reg_range(REG_IRQ_STAT, REG_IRQ_STAT),
};

static const struct regmap_access_table sn65dsi83_writeable_table = {
	.yes_ranges = sn65dsi83_writeable_ranges,
	.n_yes_ranges = ARRAY_SIZE(sn65dsi83_writeable_ranges),
};

static const struct regmap_range sn65dsi83_volatile_ranges[] = {
	regmap_reg_range(REG_RC_RESET, REG_RC_RESET),
	regmap_reg_range(REG_RC_LVDS_PLL, REG_RC_LVDS_PLL),
	regmap_reg_range(REG_IRQ_STAT, REG_IRQ_STAT),
};

static const struct regmap_access_table sn65dsi83_volatile_table = {
	.yes_ranges = sn65dsi83_volatile_ranges,
	.n_yes_ranges = ARRAY_SIZE(sn65dsi83_volatile_ranges),
};

static const struct regmap_config sn65dsi83_regmap_config = {
	.reg_bits = 8,
	.val_bits = 8,
	.rd_table = &sn65dsi83_readable_table,
	.wr_table = &sn65dsi83_writeable_table,
	.volatile_table = &sn65dsi83_volatile_table,
	.cache_type = REGCACHE_RBTREE,
	.max_register = REG_IRQ_STAT,
};

static const struct reg_default sn65dsi65_reg_defaults[] = {
	/* Reset */
	{0x09, 0x00},

	/* Core */
	{0x0A, 0x05},//ok
	{0x0B, 0x38},//ok
	{0x0D, 0x00},//ok
	{0x10, 0x28},//ok
	{0x11, 0x00},//ok
	{0x12, 0x64},//ok
	{0x13, 0x00},//ok
	{0x18, 0x6C},//ok
	{0x19, 0x0F},//ok
	{0x1A, 0x20},//ok
	{0x1B, 0x00},//ok

	/* Channel A */
	{0x20, 0x80},//ok
	{0x21, 0x07},//ok
	{0x24, 0x38},//ok
	{0x25, 0x04},//ok
	{0x28, 0x21},//ok
	{0x29, 0x00},//ok
	{0x2C, 0x1E},//ok
	{0x2D, 0x00},//ok
	{0x30, 0x04},//ok
	{0x31, 0x00},//ok
	{0x34, 0x1E},//ok
	{0x36, 0x03},//ok
	{0x38, 0x1E},//ok
	{0x3A, 0x03},//ok

	/* Channel B */
	{0x22, 0x00},//ok
	{0x23, 0x00},//ok
	{0x26, 0x00},//ok
	{0x27, 0x00},//ok
	{0x2A, 0x00},//ok
	{0x2B, 0x00},//ok
	{0x2E, 0x00},//ok
	{0x2F, 0x00},//ok
	{0x32, 0x00},//ok
	{0x33, 0x00},//ok
	{0x35, 0x00},//ok
	{0x37, 0x00},//ok
	{0x39, 0x00},//ok
	{0x3B, 0x00},//ok

	/* other settings */
	{0x3D, 0x00},//ok
	{0x3E, 0x00},//ok

	/* interrupts */
	{0xE0, 0x00},
	{0xE1, 0x00},
	{0xE2, 0x00},
	{0xE5, 0x00},
	{0xE6, 0x00},
#ifdef SN65DSI83_TEST_PATTERN
	/* Test */
	{0x3C, 0x10},
#else
	{0x3C, 0x00},
#endif
};

/* fwd declaration */
static void sn65dsi83_mode_set(struct drm_bridge *bridge,
			       const struct drm_display_mode *mode,
			       const struct drm_display_mode *adj);

static struct sn65dsi83 *bridge_to_sn65dsi83(struct drm_bridge *bridge)
{
	return container_of(bridge, struct sn65dsi83, bridge);
}

static int sn65dsi83_attach(struct drm_bridge *bridge,
			    enum drm_bridge_attach_flags flags)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
	struct device *dev = ctx->dev;
	struct mipi_dsi_device *dsi;
	struct mipi_dsi_host *host;
	int ret = 0;

	const struct mipi_dsi_device_info info = {
		.type = "sn65dsi83",
		.channel = 0,
		.node = NULL,
	};

	host = of_find_mipi_dsi_host_by_node(ctx->host_node);
	if (!host) {
		dev_err(dev, "failed to find dsi host\n");
		return -EPROBE_DEFER;
	}

	dsi = mipi_dsi_device_register_full(host, &info);
	if (IS_ERR(dsi)) {
		return dev_err_probe(dev, PTR_ERR(dsi),
				     "failed to create dsi device\n");
	}

	ctx->dsi = dsi;

	dsi->lanes = ctx->dsi_lanes;
	dsi->format = MIPI_DSI_FMT_RGB888;
	dsi->mode_flags = MIPI_DSI_MODE_VIDEO /*| MIPI_DSI_MODE_VIDEO_BURST*/;

	ret = mipi_dsi_attach(dsi);
	if (ret < 0) {
		dev_err(dev, "failed to attach dsi to host\n");
		goto err_dsi_attach;
	}

	return drm_bridge_attach(bridge->encoder, ctx->panel_bridge,
				 &ctx->bridge, flags);

err_dsi_attach:
	mipi_dsi_device_unregister(dsi);
	return ret;
}

static void sn65dsi83_pre_enable(struct drm_bridge *bridge)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);

	/*
	 * Reset the chip, pull EN line low for t_reset=10ms,
	 * then high for t_en=1ms.
	 */
	regcache_mark_dirty(ctx->regmap);
	gpiod_set_value(ctx->enable_gpio, 0);
	usleep_range(10000, 11000);
	gpiod_set_value(ctx->enable_gpio, 1);
	usleep_range(1000, 1100);

	/* disable PLL, should already be the case after toggle of enable pin */
	regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
	regmap_write(ctx->regmap, REG_RC_RESET, 0x00);

#ifdef MODE_HACK
	/* TODO: hack until mode_set and mode_valid are called */
	struct drm_display_mode *adjusted_mode =
		&(bridge->encoder->crtc->state->adjusted_mode);
	sn65dsi83_mode_set(bridge, &ctx->mode, adjusted_mode);
	ctx->lvds_format_24bpp = true;
	ctx->lvds_format_jeida = false;
#endif
}

static u8 sn65dsi83_get_lvds_range(struct sn65dsi83 *ctx)
{
	/*
	 * The encoding of the LVDS_CLK_RANGE is as follows:
	 * 000 - 25 MHz <= LVDS_CLK < 37.5 MHz
	 * 001 - 37.5 MHz <= LVDS_CLK < 62.5 MHz
	 * 010 - 62.5 MHz <= LVDS_CLK < 87.5 MHz
	 * 011 - 87.5 MHz <= LVDS_CLK < 112.5 MHz
	 * 100 - 112.5 MHz <= LVDS_CLK < 137.5 MHz
	 * 101 - 137.5 MHz <= LVDS_CLK <= 154 MHz
	 * which is a range of 12.5MHz..162.5MHz in 50MHz steps, except that
	 * the ends of the ranges are clamped to the supported range. Since
	 * sn65dsi83_mode_valid() already filters the valid modes and limits
	 * the clock to 25..154 MHz, the range calculation can be simplified
	 * as follows:
	 */
	int mode_clock = ctx->mode.clock;

	if (ctx->lvds_dual_link)
		mode_clock /= 2;

	return (mode_clock - 12500) / 25000;
}

static u8 sn65dsi83_get_dsi_range(struct sn65dsi83 *ctx)
{
	/*
	 * The encoding of the CHA_DSI_CLK_RANGE is as follows:
	 * 0x00 through 0x07 - Reserved
	 * 0x08 - 40 <= DSI_CLK < 45 MHz
	 * 0x09 - 45 <= DSI_CLK < 50 MHz
	 * ...
	 * 0x63 - 495 <= DSI_CLK < 500 MHz
	 * 0x64 - 500 MHz
	 * 0x65 through 0xFF - Reserved
	 * which is DSI clock in 5 MHz steps, clamped to 40..500 MHz.
	 * The DSI clock are calculated as:
	 *  DSI_CLK = mode clock * bpp / dsi_data_lanes / 2
	 * the 2 is there because the bus is DDR.
	 */

	u8 dsiClk = DIV_ROUND_UP(clamp(((unsigned int)ctx->mode.clock *
			    mipi_dsi_pixel_format_to_bpp(ctx->dsi->format)) /
			    (ctx->dsi_lanes * 2), 40000U, 500000U), 5000U);
	printk(KERN_ERR "TIM: %s: set dsiClk between %d and %d Mhz based on %ld, %d, %d\n",
	       __func__, dsiClk * 5, dsiClk * 6, ctx->mode.clock, ctx->dsi->format, ctx->dsi_lanes);
	return dsiClk;
}

static u8 sn65dsi83_get_dsi_div(struct sn65dsi83 *ctx)
{
	/* The divider is (DSI_CLK / LVDS_CLK) - 1, which really is: */
	unsigned int dsi_div = mipi_dsi_pixel_format_to_bpp(ctx->dsi->format);

	dsi_div /= ctx->dsi_lanes;

	if (!ctx->lvds_dual_link)
		dsi_div /= 2;

	printk(KERN_ERR "TIM: %s: set dsiDiv to %d based on %ld, %d, %d\n",
	       __func__, dsi_div -1, ctx->dsi->format, ctx->dsi_lanes, ctx->lvds_dual_link);
	return dsi_div - 1;
}

static void dumpRegs(struct drm_bridge *bridge)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
	unsigned int val;
	int i;

	for (i = 0; i < ARRAY_SIZE(sn65dsi65_reg_defaults); i++) {
		struct reg_default conf = sn65dsi65_reg_defaults[i];
		regmap_read(ctx->regmap, conf.reg, &val);
		printk(KERN_ERR "TIM: %s: reg 0x%02x val 0x%02x\n", __func__, conf.reg, val);
	}
}

static void sn65dsi83_enable(struct drm_bridge *bridge)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
	unsigned int pval;
	u16 val;
	int ret, i;

	/* Reference clock derived from DSI link clock. */
	regmap_write(ctx->regmap, REG_RC_LVDS_PLL,
		REG_RC_LVDS_PLL_LVDS_CLK_RANGE(sn65dsi83_get_lvds_range(ctx)) |
		REG_RC_LVDS_PLL_HS_CLK_SRC_DPHY);
	regmap_write(ctx->regmap, REG_DSI_CLK,
		REG_DSI_CLK_CHA_DSI_CLK_RANGE(sn65dsi83_get_dsi_range(ctx)));
	regmap_write(ctx->regmap, REG_RC_DSI_CLK,
		REG_RC_DSI_CLK_DSI_CLK_DIVIDER(sn65dsi83_get_dsi_div(ctx)));

	/* Set number of DSI lanes and LVDS link config. */
	regmap_write(ctx->regmap, REG_DSI_LANE,
		REG_DSI_LANE_DSI_CHANNEL_MODE_SINGLE |
		REG_DSI_LANE_CHA_DSI_LANES(~(ctx->dsi_lanes - 1)) |
		/* CHB is DSI85-only, set to default on DSI83/DSI84 */
		REG_DSI_LANE_CHB_DSI_LANES(3));
	/* No equalization. */
	regmap_write(ctx->regmap, REG_DSI_EQ, 0x00);

	/* Set up sync signal polarity. */
	val = (ctx->mode.flags & DRM_MODE_FLAG_NHSYNC ?
	       REG_LVDS_FMT_HS_NEG_POLARITY : 0) |
	      (ctx->mode.flags & DRM_MODE_FLAG_NVSYNC ?
	       REG_LVDS_FMT_VS_NEG_POLARITY : 0);

	/* Set up bits-per-pixel, 18bpp or 24bpp. */
	if (ctx->lvds_format_24bpp) {
		val |= REG_LVDS_FMT_CHA_24BPP_MODE;
		if (ctx->lvds_dual_link)
			val |= REG_LVDS_FMT_CHB_24BPP_MODE;
	}

	/* Set up LVDS format, JEIDA/Format 1 or SPWG/Format 2 */
	if (ctx->lvds_format_jeida) {
		val |= REG_LVDS_FMT_CHA_24BPP_FORMAT1;
		if (ctx->lvds_dual_link)
			val |= REG_LVDS_FMT_CHB_24BPP_FORMAT1;
	}

	/* Set up LVDS output config (DSI84,DSI85) */
	if (!ctx->lvds_dual_link)
		val |= REG_LVDS_FMT_LVDS_LINK_CFG;

	regmap_write(ctx->regmap, REG_LVDS_FMT, val);
	// set correct voltage swing for InnoLux, TODO: configure through DTS
	//regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x05);
	regmap_write(ctx->regmap, REG_LVDS_VCOM, 0x0F);
	// set 100ohm term for InnoLux, TODO: configure through DTS, using 200hm as default and optionally adding '100-ohm-termination;'
	// set reverse channel A LVDS, TODO: configure through DTS, using non-reverse as default and optionally adding 'reverse-channel-A;' and/or 'reverse-channel-B;'
	/*regmap_write(ctx->regmap, REG_LVDS_LANE,
		(ctx->lvds_dual_link_even_odd_swap ?
		 REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
		REG_LVDS_LANE_CHA_LVDS_TERM |
		REG_LVDS_LANE_CHB_LVDS_TERM);*/
	regmap_write(ctx->regmap, REG_LVDS_LANE,
		(ctx->lvds_dual_link_even_odd_swap ?
		 REG_LVDS_LANE_EVEN_ODD_SWAP : 0) |
		 REG_LVDS_LANE_CHA_REVERSE_LVDS);
	regmap_write(ctx->regmap, REG_LVDS_CM, 0x00);

	// only single channel DSI is supported for now
	val = ctx->mode.hdisplay;
	regmap_bulk_write(ctx->regmap, REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
			&val, 2);
	regmap_bulk_write(ctx->regmap, REG_VID_CHA_VERTICAL_DISPLAY_SIZE_LOW,
			&ctx->mode.vdisplay, 2);

	val = 32 + 1;	/* 32 + 1 pixel clock to ensure proper operation */
	regmap_bulk_write(ctx->regmap, REG_VID_CHA_SYNC_DELAY_LOW, &val, 2);

	printk(KERN_ERR "TIM: %s: hsync_end %d hsync_start %d\n", __func__, ctx->mode.hsync_end, ctx->mode.hsync_start);
	val = ctx->mode.hsync_end - ctx->mode.hsync_start;
	if (ctx->lvds_dual_link)
		val /= 2;
	regmap_bulk_write(ctx->regmap, REG_VID_CHA_HSYNC_PULSE_WIDTH_LOW,
			  &val, 2);
	
	printk(KERN_ERR "TIM: %s: vsync_end %d vsync_start %d\n", __func__, ctx->mode.vsync_end, ctx->mode.vsync_start);
	val = ctx->mode.vsync_end - ctx->mode.vsync_start;
	regmap_bulk_write(ctx->regmap, REG_VID_CHA_VSYNC_PULSE_WIDTH_LOW,
			  &val, 2);

	printk(KERN_ERR "TIM: %s: htotal %d hsync_end %d\n", __func__, ctx->mode.htotal, ctx->mode.hsync_end);
	val = ctx->mode.htotal - ctx->mode.hsync_end;
	if (ctx->lvds_dual_link)
		val /= 2;
	regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_BACK_PORCH,
		     val);
	
	printk(KERN_ERR "TIM: %s: vtotal %d vsync_end %d\n", __func__, ctx->mode.vtotal, ctx->mode.vsync_end);
	val = ctx->mode.vtotal - ctx->mode.vsync_end;
	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_BACK_PORCH,
		     val);

	printk(KERN_ERR "TIM: %s: hsync_start %d hdisplay %d\n", __func__, ctx->mode.hsync_start, ctx->mode.hdisplay);
	val = ctx->mode.hsync_start - ctx->mode.hdisplay;
	if (ctx->lvds_dual_link)
		val /= 2;
	regmap_write(ctx->regmap, REG_VID_CHA_HORIZONTAL_FRONT_PORCH,
		     val);
	
	printk(KERN_ERR "TIM: %s: vsync_start %d vdisplay %d\n", __func__, ctx->mode.vsync_start, ctx->mode.vdisplay);
	val = ctx->mode.vsync_start - ctx->mode.vdisplay;
	regmap_write(ctx->regmap, REG_VID_CHA_VERTICAL_FRONT_PORCH,
		     val);
	//enable test pattern
#ifndef SN65DSI83_TEST_PATTERN
	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x00);
#else
	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x10);
#endif

	dumpRegs(bridge);

#ifdef HARDCODED_REGS
	printk(KERN_ERR "TIM: %s: sn65dsi65_reg_defaults\n", __func__);
	for (i = 0; i < ARRAY_SIZE(sn65dsi65_reg_defaults); i++) {
		struct reg_default conf = sn65dsi65_reg_defaults[i];
		regmap_write(ctx->regmap, conf.reg, conf.def);
	}
	printk(KERN_ERR "TIM: %s: written %d sn65dsi65_reg_defaults\n",
	       __func__, i);
#endif

	/* Enable PLL */
	regmap_write(ctx->regmap, REG_RC_PLL_EN, REG_RC_PLL_EN_PLL_EN);
	usleep_range(3000, 4000);
	ret = regmap_read_poll_timeout(ctx->regmap, REG_RC_LVDS_PLL, pval,
					pval & REG_RC_LVDS_PLL_PLL_EN_STAT,
					1000, 100000);
	if (ret) {
		dev_err(ctx->dev, "failed to lock PLL, ret=%i\n", ret);
		/* On failure, disable PLL again and exit. */
		regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
		return;
	}

	/* Trigger reset after CSR register update. */
	regmap_write(ctx->regmap, REG_RC_RESET, REG_RC_RESET_SOFT_RESET);

	/* Clear all errors that got asserted during initialization. */
	regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
	regmap_write(ctx->regmap, REG_IRQ_STAT, pval);
}

static void sn65dsi83_disable(struct drm_bridge *bridge)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);

	/* Clear reset, disable PLL */
	regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
	regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
}

static void sn65dsi83_post_disable(struct drm_bridge *bridge)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);

	/* Put the chip in reset, pull EN line low. */
	gpiod_set_value(ctx->enable_gpio, 0);
}

static enum drm_mode_status
sn65dsi83_mode_valid(struct drm_bridge *bridge,
		     const struct drm_display_info *info,
		     const struct drm_display_mode *mode)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);

	/* LVDS output clock range 25..154 MHz */
	if (mode->clock < 25000)
		return MODE_CLOCK_LOW;
	if (mode->clock > 154000)
		return MODE_CLOCK_HIGH;

	switch (info->bus_formats[0]) {
	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
		ctx->lvds_format_24bpp = false;
		ctx->lvds_format_jeida = false;
		break;
	case MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA:
		ctx->lvds_format_24bpp = true;
		ctx->lvds_format_jeida = true;
		break;
	case MEDIA_BUS_FMT_RGB888_1X7X4_SPWG:
		ctx->lvds_format_24bpp = true;
		ctx->lvds_format_jeida = false;
		break;
	default:
		ctx->lvds_format_24bpp = true;
		ctx->lvds_format_jeida = false;
		/*
		 * Some bridges still don't set the correct LVDS bus pixel
		 * format, use SPWG24 default format until those are fixed.
		 */
		dev_warn(ctx->dev,
			"Unsupported LVDS bus format 0x%04x, please check output bridge driver. Falling back to SPWG24.\n",
			info->bus_formats[0]);
		break;
	}

	printk(KERN_ERR "TIM: %s: mode %ux%u@%d is valid\n", __func__, mode->hdisplay, mode->vdisplay, mode->clock);

	return MODE_OK;
}

static void sn65dsi83_mode_set(struct drm_bridge *bridge,
			       const struct drm_display_mode *mode,
			       const struct drm_display_mode *adj)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
	u32 input_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
	struct drm_encoder *encoder = bridge->encoder;
	struct drm_device *ddev = encoder->dev;
	struct drm_connector *connector;

	/* The DSI format is always RGB888_1X24 */
	list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
		if (connector->encoder != encoder)
			continue;

		drm_display_info_set_bus_formats(&connector->display_info,
						 &input_bus_format, 1);
	}

	ctx->mode = *adj;
}

static const struct drm_bridge_funcs sn65dsi83_funcs = {
	.attach		= sn65dsi83_attach,
	.pre_enable	= sn65dsi83_pre_enable,
	.enable		= sn65dsi83_enable,
	.disable	= sn65dsi83_disable,
	.post_disable	= sn65dsi83_post_disable,
	.mode_valid	= sn65dsi83_mode_valid,
	.mode_set	= sn65dsi83_mode_set,
};

static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
{
	struct drm_bridge *panel_bridge;
	struct device *dev = ctx->dev;
	struct device_node *endpoint;
	struct drm_panel *panel;
	int ret;

	endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);

	printk(KERN_ERR "TIM: %s: found endpoint %s\n",
	       __func__, endpoint->full_name ? endpoint->full_name: endpoint->name);

	//ctx->lvds_termination = of_property_read_u32(endpoint, "lvds-term");
	ctx->dsi_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
	ctx->host_node = of_graph_get_remote_port_parent(endpoint);
	of_node_put(endpoint);

	printk(KERN_ERR "TIM: %s: dsi_lanes count is %d\n", __func__, ctx->dsi_lanes);

	if (ctx->dsi_lanes < 0 || ctx->dsi_lanes > 4)
		return -EINVAL;
	if (!ctx->host_node)
		return -ENODEV;

	ctx->lvds_dual_link = false;
	ctx->lvds_dual_link_even_odd_swap = false;
	if (model != MODEL_SN65DSI83) {
		struct device_node *port2, *port3;
		int dual_link;

		port2 = of_graph_get_port_by_id(dev->of_node, 2);
		port3 = of_graph_get_port_by_id(dev->of_node, 3);
		dual_link = drm_of_lvds_get_dual_link_pixel_order(port2, port3);
		of_node_put(port2);
		of_node_put(port3);

		if (dual_link == DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS) {
			ctx->lvds_dual_link = true;
			/* Odd pixels to LVDS Channel A, even pixels to B */
			ctx->lvds_dual_link_even_odd_swap = false;
		} else if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) {
			ctx->lvds_dual_link = true;
			/* Even pixels to LVDS Channel A, odd pixels to B */
			ctx->lvds_dual_link_even_odd_swap = true;
		}
	}

	ret = drm_of_find_panel_or_bridge(dev->of_node, 2, 0, &panel, &panel_bridge);
	if (ret < 0)
		return ret;

	if (panel) {
		printk(KERN_ERR "TIM: %s: panel found\n", __func__);
		if (panel->dev) {
			if (panel->dev->of_node) {
				printk(KERN_ERR "TIM: %s: panel of_node %s\n",
				       __func__, panel->dev->of_node->full_name);
			}
			printk(KERN_ERR "TIM: %s: panel dev %s\n",
			       __func__, panel->dev->init_name);
		} else {
			printk(KERN_ERR "TIM: %s: panel invalid dev/of_node\n",
			       __func__);
		}
		panel_bridge = devm_drm_panel_bridge_add(dev, panel);
		if (IS_ERR(panel_bridge))
			return PTR_ERR(panel_bridge);
	}

	ctx->panel_bridge = panel_bridge;

	return 0;
}

static int sn65dsi83_probe(struct i2c_client *client,
			   const struct i2c_device_id *id)
{
	struct device *dev = &client->dev;
	enum sn65dsi83_model model;
	struct sn65dsi83 *ctx;
	int ret;

	printk(KERN_ERR "TIM: %s: init\n", __func__);

	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
	if (!ctx)
		return -ENOMEM;

	ctx->dev = dev;

	if (dev->of_node)
		model = (enum sn65dsi83_model)of_device_get_match_data(dev);
	else
		model = id->driver_data;

	ctx->enable_gpio = devm_gpiod_get(ctx->dev, "enable", GPIOD_OUT_LOW);
	if (IS_ERR(ctx->enable_gpio))
		return PTR_ERR(ctx->enable_gpio);

	ret = sn65dsi83_parse_dt(ctx, model);
	if (ret)
		return ret;

	ctx->regmap = devm_regmap_init_i2c(client, &sn65dsi83_regmap_config);
	if (IS_ERR(ctx->regmap))
		return PTR_ERR(ctx->regmap);

	dev_set_drvdata(dev, ctx);
	i2c_set_clientdata(client, ctx);

	ctx->bridge.funcs = &sn65dsi83_funcs;
	ctx->bridge.of_node = dev->of_node;
	drm_bridge_add(&ctx->bridge);

	printk(KERN_ERR "TIM: %s: exit\n", __func__);

	return 0;
}

static int sn65dsi83_remove(struct i2c_client *client)
{
	struct sn65dsi83 *ctx = i2c_get_clientdata(client);

	mipi_dsi_detach(ctx->dsi);
	mipi_dsi_device_unregister(ctx->dsi);
	drm_bridge_remove(&ctx->bridge);
	of_node_put(ctx->host_node);

	return 0;
}

static struct i2c_device_id sn65dsi83_id[] = {
	{ "ti,sn65dsi83", MODEL_SN65DSI83 },
	{ "ti,sn65dsi84", MODEL_SN65DSI84 },
	{},
};
MODULE_DEVICE_TABLE(i2c, sn65dsi83_id);

static const struct of_device_id sn65dsi83_match_table[] = {
	{ .compatible = "ti,sn65dsi83", .data = (void *)MODEL_SN65DSI83 },
	{ .compatible = "ti,sn65dsi84", .data = (void *)MODEL_SN65DSI84 },
	{},
};
MODULE_DEVICE_TABLE(of, sn65dsi83_match_table);

static struct i2c_driver sn65dsi83_driver = {
	.probe = sn65dsi83_probe,
	.remove = sn65dsi83_remove,
	.id_table = sn65dsi83_id,
	.driver = {
		.name = "sn65dsi83",
		.of_match_table = sn65dsi83_match_table,
	},
};
module_i2c_driver(sn65dsi83_driver);

MODULE_AUTHOR("Marek Vasut <marex@denx.de>");
MODULE_DESCRIPTION("TI SN65DSI83 DSI to LVDS bridge driver");
MODULE_LICENSE("GPL v2");

milo246
Posts: 73
Joined: Mon Mar 01, 2021 1:43 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Mon May 17, 2021 1:26 pm

Okay, compiled and running now. No image.

Main problem is apparently that all of the timing parameters are zeroed out, so that's never a good thing. Probably the data got lost somewhere or some callback isn't arriving... brr... I think I saw something on that in this huge thread, so I have some scrolling to do...

The clock and other setup registers are also subtly different. Pff.

Code: Select all

$ diff -y -W 22 registers-marek registers-jagan 
00: 35		00: 35
01: 38		01: 38
02: 49		02: 49
03: 53		03: 53
04: 44		04: 44
05: 20		05: 20
06: 20		06: 20
07: 20		07: 20
08: 01		08: 01
09: 01	  |	09: 00
0a: 01	  |	0a: 83
0b: 28		0b: 28
0d: 01		0d: 01
10: 36		10: 36
11: 00		11: 00
12: 08	  |	12: 3c
18: 18	  |	18: 78
19: 05		19: 05
1a: 03		1a: 03
1b: 00		1b: 00
20: 00		20: 00
21: 00	  |	21: 04
24: 00	  |	24: 58
25: 00	  |	25: 02
28: 21		28: 21
29: 00		29: 00
2c: 00	  |	2c: 01
2d: 00		2d: 00
30: 00	  |	30: 01
31: 00		31: 00
34: 00	  |	34: a0
36: 00	  |	36: 17
38: 00	  |	38: 7f
3a: 00	  |	3a: 0b
3c: 00		3c: 00
e0: 00		e0: 00
e1: 00		e1: 00
e5: 0d	  |	e5: 00

UberDoefus
Posts: 19
Joined: Tue May 04, 2021 8:28 am

Re: Rpi 4 with DRM and 7inch panel using kms driver

Mon May 17, 2021 1:43 pm

milo246 wrote:
Mon May 17, 2021 1:26 pm
Okay, compiled and running now. No image.

Main problem is apparently that all of the timing parameters are zeroed out, so that's never a good thing. Probably the data got lost somewhere or some callback isn't arriving... brr... I think I saw something on that in this huge thread, so I have some scrolling to do...

The clock and other setup registers are also subtly different. Pff.

Code: Select all

$ diff -y -W 22 registers-marek registers-jagan 
00: 35		00: 35
01: 38		01: 38
02: 49		02: 49
03: 53		03: 53
04: 44		04: 44
05: 20		05: 20
06: 20		06: 20
07: 20		07: 20
08: 01		08: 01
09: 01	  |	09: 00
0a: 01	  |	0a: 83
0b: 28		0b: 28
0d: 01		0d: 01
10: 36		10: 36
11: 00		11: 00
12: 08	  |	12: 3c
18: 18	  |	18: 78
19: 05		19: 05
1a: 03		1a: 03
1b: 00		1b: 00
20: 00		20: 00
21: 00	  |	21: 04
24: 00	  |	24: 58
25: 00	  |	25: 02
28: 21		28: 21
29: 00		29: 00
2c: 00	  |	2c: 01
2d: 00		2d: 00
30: 00	  |	30: 01
31: 00		31: 00
34: 00	  |	34: a0
36: 00	  |	36: 17
38: 00	  |	38: 7f
3a: 00	  |	3a: 0b
3c: 00		3c: 00
e0: 00		e0: 00
e1: 00		e1: 00
e5: 0d	  |	e5: 00
Well, I still have a nasty hack in there for the missing mode callbacks and hardcoded stuff for my panel in there. This is something that needs to be fixed somehow, don't know if this causes your issue.
In
sn65dsi83_pre_enable
I manually call the
sn65dsi83_mode_set
, if not, the
ctx->mode.clock
remains 0.

My team is planning to add the 100ohm/voltage swing and reverse selection to the DTS parameters so that this becomes generic iso hardcoded. We're attempting to contact Marek about this addition, as well as the correction for dual-lvds hback-porch and front-porch.

milo246
Posts: 73
Joined: Mon Mar 01, 2021 1:43 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Mon May 17, 2021 2:03 pm

Indeed, set_mode is never called so "mode" remains all zeroes.

Simple workaround it putting this in the enable method:

Code: Select all

	/* set_mode is never called, so do that here... */
	if (!ctx->mode.clock) {
		dev_info(ctx->dev, "%s set_mode fixup\n", __func__);
		ctx->mode = bridge->encoder->crtc->state->adjusted_mode;
	}
And then it works. And the register contents are identical too (apart from the "reset" bit)

Code: Select all

$ diff -y -W 22 registers-marek registers-jagan 
00: 35		00: 35
01: 38		01: 38
02: 49		02: 49
03: 53		03: 53
04: 44		04: 44
05: 20		05: 20
06: 20		06: 20
07: 20		07: 20
08: 01		08: 01
09: 01	  |	09: 00
0a: 83		0a: 83
0b: 28		0b: 28
0d: 01		0d: 01
10: 36		10: 36
11: 00		11: 00
12: 3c		12: 3c
18: 78		18: 78
19: 05		19: 05
1a: 03		1a: 03
1b: 00		1b: 00
20: 00		20: 00
21: 04		21: 04
24: 58		24: 58
25: 02		25: 02
28: 21		28: 21
29: 00		29: 00
2c: 01		2c: 01
2d: 00		2d: 00
30: 01		30: 01
31: 00		31: 00
34: a0		34: a0
36: 17		36: 17
38: 7f		38: 7f
3a: 0b		3a: 0b
3c: 00		3c: 00
e0: 00		e0: 00
e1: 00		e1: 00
e5: 00		e5: 00

UberDoefus
Posts: 19
Joined: Tue May 04, 2021 8:28 am

Re: Rpi 4 with DRM and 7inch panel using kms driver

Mon May 17, 2021 4:09 pm

milo246 wrote:
Mon May 17, 2021 2:03 pm
Indeed, set_mode is never called so "mode" remains all zeroes.

Simple workaround it putting this in the enable method:

Code: Select all

	/* set_mode is never called, so do that here... */
	if (!ctx->mode.clock) {
		dev_info(ctx->dev, "%s set_mode fixup\n", __func__);
		ctx->mode = bridge->encoder->crtc->state->adjusted_mode;
	}
And then it works. And the register contents are identical too (apart from the "reset" bit)

Code: Select all

$ diff -y -W 22 registers-marek registers-jagan 
00: 35		00: 35
01: 38		01: 38
02: 49		02: 49
03: 53		03: 53
04: 44		04: 44
05: 20		05: 20
06: 20		06: 20
07: 20		07: 20
08: 01		08: 01
09: 01	  |	09: 00
0a: 83		0a: 83
0b: 28		0b: 28
0d: 01		0d: 01
10: 36		10: 36
11: 00		11: 00
12: 3c		12: 3c
18: 78		18: 78
19: 05		19: 05
1a: 03		1a: 03
1b: 00		1b: 00
20: 00		20: 00
21: 04		21: 04
24: 58		24: 58
25: 02		25: 02
28: 21		28: 21
29: 00		29: 00
2c: 01		2c: 01
2d: 00		2d: 00
30: 01		30: 01
31: 00		31: 00
34: a0		34: a0
36: 17		36: 17
38: 7f		38: 7f
3a: 0b		3a: 0b
3c: 00		3c: 00
e0: 00		e0: 00
e1: 00		e1: 00
e5: 00		e5: 00
That's why in my driver I have the

Code: Select all

#define MODE_HACK
to call both mode functions in the pre-enable, I thought you're using my version of the driver...

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Tue May 18, 2021 7:31 am

Been busy yesterday, so after having made preparations on the weekend I've started this morning.

Starting point:
- kernel image modified by 6by9 (https://github.com/6by9/linux/tree/rpi- ... si8x-marek ) ; (note: used that for milo246 testing as well)

Code: Select all

pi@raspberrypi:~ $ uname -a
Linux raspberrypi 5.10.33-v7l+ #1 SMP Sun May 16 11:19:24 CEST 2021 armv7l GNU/Linux
- this image has marek V3 driver, so I've patched it to V4
- compiled the full kernel with DSI84 and PWM backlight driver enabled in the build

So, today I've tested the result: no image with default V4 driver

I've then made some changes which were related to the init_sequence, mode_set call and finally on media-18 handling (as I've previously mentioned the driver will fail on jeida-18) and all this brought me to here:
IMG_20210518_081046.jpg
Marek V4 driver running, but still issues with mapping (unsolved atm!)
IMG_20210518_081046.jpg (212.9 KiB) Viewed 927 times
So, the driver is able to run, but there are still unsolved issues coming from the data mapping, or the format specifiers.
I need to dig a little further and try to figure out why those guys have this strange 'MEDIA_BUS_FMT_RGB_1x24' format specifier in their 'sn65dsi83_mode_set' function.

Code: Select all

static void sn65dsi83_mode_set(struct drm_bridge *bridge,
			       const struct drm_display_mode *mode,
			       const struct drm_display_mode *adj)
{
	struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
	u32 input_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
	struct drm_encoder *encoder = bridge->encoder;
	struct drm_device *ddev = encoder->dev;
	struct drm_connector *connector;

	/* The DSI format is always RGB888_1X24 */
	list_for_each_entry(connector, &ddev->mode_config.connector_list, head) {
		if (connector->encoder != encoder)
			continue;

		drm_display_info_set_bus_formats(&connector->display_info,
						 &input_bus_format, 1);
	}

	ctx->mode = *adj;
}
When dealing with DSI the mode should be a DSI mode ('MIPI_DSI_FMT_RGB888') as used in 'sn65dsi83_attach' function. So, there is the risk the above is mandatory for their i.MX platform they are testing with but incorrect elsewhere.
Will come back - hopefully later today ...

My current overlay and modified Marek V4 driver source can be found from the attachment. I've added some small comment were I've changed the code.
Attachments
ti-sn65dsi83_mod2_abgwn.tar
change V4 sources which result in what is pictured above
(30 KiB) Downloaded 14 times
Last edited by aBUGSworstnightmare on Tue May 18, 2021 7:39 am, edited 1 time in total.

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Tue May 18, 2021 7:37 am

@milo246:
I think I've figured out another issue when looking at your driver sources. In the disable routine you've missed to toggle the SOFT_RESET bit. not doing so will not stop the DLL --> can be the possible root cause for the display not coming back from screen blanking (not tested yet).
So you may want to change to as SOFT_RESET needs to be toggled after CSR update

Code: Select all

static void sn65dsi_disable(struct drm_bridge *bridge)
{
	struct sn65dsi *sn = bridge_to_sn65dsi(bridge);

	dev_info(sn->dev, "%s\n", __func__);

	/* unset PLL_EN bit */
	regmap_write(sn->regmap, SN65DSI_CLK_PLL, 0x0);
	/* set SOFT_RESET bit */
	regmap_write(sn->regmap, SN65DSI_SOFT_RESET, SOFT_RESET);
	usleep_range(3000, 4000);
}

milo246
Posts: 73
Joined: Mon Mar 01, 2021 1:43 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Tue May 18, 2021 8:43 am

My current view on this is to just assert the "reset" signal at that point, so the chip is powered down.

milo246
Posts: 73
Joined: Mon Mar 01, 2021 1:43 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Tue May 18, 2021 9:07 am

I updated to v4 and also get garbled colors. That is because sn65dsi83_mode_valid is never called, so the format setting is borked. Guess it needs the same workaround as set_mode.

(edit) Indeed. Same workaround as for mode_set applies, then the colors are okay again.

Attached the patches (including marek's) that I apply on the kernel, plus the resulting driver file.
Attachments
ti-sn65dsi83-marek-v4-with-rpi4-workarounds.tar.gz
(11.16 KiB) Downloaded 19 times

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Tue May 18, 2021 10:09 am

milo246 wrote:
Tue May 18, 2021 8:43 am
My current view on this is to just assert the "reset" signal at that point, so the chip is powered down.
Have you checked that you pull enable line low? Don't see this from the code --> PLL continues and re-enable will fail.
Will check your code later! Thanks for the update and feedback!

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Tue May 18, 2021 1:40 pm

o.k. .. could not resist to make some changes and add some comments.
And yes, will also not get tired to fix incorrect config for "jeida-18" modules. I've added two pictures with descriptions on settings.
IMG_20210518_150534.jpg
Incorrect register setting for JEIDA-18 module.
This is the output when CHx_24BPP_FORMAT1 is set to zero (when it should be 1 = FORMAT1). CHx_24BPP_MODE is zero as there are 18-bit LVDS data only
IMG_20210518_150534.jpg (167.01 KiB) Viewed 860 times
IMG_20210518_150214.jpg
Correct register setting for JEIDA-18 module.
This is the output when CHx_24BPP_FORMAT1 is set to one = FORMAT1. CHx_24BPP_MODE is zero as there are 18-bit LVDS data only
IMG_20210518_150214.jpg (109.64 KiB) Viewed 860 times
Find my modified source file from below.

EDIT: will not restart the display once blanking kicked in! Same as experienced with milo246 driver.
Attachments
ti-sn65dsi83_aBUGSworstnightmare.tar
modified driver file which fixes incorrect settings for JEIDA-18 modules.
(30 KiB) Downloaded 14 times

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 11246
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Rpi 4 with DRM and 7inch panel using kms driver

Thu May 27, 2021 3:47 pm

Marek's v4 has now been pushed to https://github.com/6by9/linux/tree/rpi- ... i8x-marek/, along with a rework to vc4_dsi.c that uses pm_runtime to power up the PHY and reinstate the framework calling mode_fixup/mode_valid/mode_set functions.

I've validated it as far as it loads the driver and calls all the right sn65dsi83 functions.
If the chip really doesn't like video having started during initialisation then I think the DRM framework hits an issue as it wants hooks in states that the frameworks don't provide. If it were initialised over the DSI bus then the use of the DSI transfer function would power the PHY up with a delayed autosuspend, but sn65dsi8[3|4] is I2C based.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 4:51 am

6by9 wrote:
Thu May 27, 2021 3:47 pm
Marek's v4 has now been pushed to https://github.com/6by9/linux/tree/rpi- ... i8x-marek/, along with a rework to vc4_dsi.c that uses pm_runtime to power up the PHY and reinstate the framework calling mode_fixup/mode_valid/mode_set functions.

I've validated it as far as it loads the driver and calls all the right sn65dsi83 functions.
If the chip really doesn't like video having started during initialisation then I think the DRM framework hits an issue as it wants hooks in states that the frameworks don't provide. If it were initialised over the DSI bus then the use of the DSI transfer function would power the PHY up with a delayed autosuspend, but sn65dsi8[3|4] is I2C based.
hi 6by9, thanks for your support/feedback. Let me try it today and report back.
Edit1: pulled the repro, changed DSI lane config in overlay to 3 (4-lane didn't work so far, so will test later; 3 is a save start), enabled the bridge and PWM via menuconfig and started compiling...

Edit2: instantly worked after kernel compile/install with the display mode defined as JEIDA-24 (MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA; that's what the current setting in panel-simple are).
IMG_20210528_094852.jpg
IMG_20210528_094852.jpg (132.3 KiB) Viewed 574 times
IMG_20210528_094847.jpg
IMG_20210528_094847.jpg (117.35 KiB) Viewed 574 times
Last edited by aBUGSworstnightmare on Fri May 28, 2021 8:28 am, edited 1 time in total.

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 8:27 am

Driver still has wrong coding in case panel mode is JEIDA-18 (MEDIA_BUS_FMT_RGB666_1X7X3_SPWG) so fails.
IMG_20210528_095431.jpg
IMG_20210528_095431.jpg (203.57 KiB) Viewed 573 times
IMG_20210528_095401.jpg
IMG_20210528_095401.jpg (136.55 KiB) Viewed 573 times
IMG_20210528_095415.jpg
IMG_20210528_095415.jpg (154.67 KiB) Viewed 573 times
According to https://elixir.bootlin.com/linux/latest ... vds.c#L160 "jeida-18" is MEDIA_BUS_FMT_RGB666_1X7X3_SPWG. So the driver needs to enable 'FORMAT1' for JEIDA but doesn't.
Same fix as applied previously; lvds_format_jeida needs to be TRUE :

Code: Select all

	case MEDIA_BUS_FMT_RGB666_1X7X3_SPWG:
		ctx->lvds_format_24bpp = false;
		ctx->lvds_format_jeida = true;
Will have to check how the driver acts in case of screen blanking .. will report later.

milo246
Posts: 73
Joined: Mon Mar 01, 2021 1:43 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 8:48 am

I think the bug is that the driver does not take the input (DSI mode) into account.

For MEDIA_BUS_FMT_RGB666_1X7X3_SPWG output:

If the DSI link is sending 18-bit data, then jeida=false and 24bpp=false
If the DSI link is sending 24-bit data, then jeida=true and 24bpp=false

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 9:20 am

milo246 wrote:
Fri May 28, 2021 8:48 am
I think the bug is that the driver does not take the input (DSI mode) into account.

For MEDIA_BUS_FMT_RGB666_1X7X3_SPWG output:

If the DSI link is sending 18-bit data, then jeida=false and 24bpp=false
If the DSI link is sending 24-bit data, then jeida=true and 24bpp=false
it's not related to DSI here. It's the mode defined for the panel (in panel-simple.c) as the DSI link can keep sending 24-bit; no need to send 18bit data only because the bridge will happily convert 24-bit input to 18-bit output. Way simpler than configurimg the mode for in- and output.
Sure, if you want to reduce DSI throughput to the minimum you need to consider this. But your driver is also always sending 24-bit data or have you changed that meanwhile?

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 9:32 am

Driver still fails on screen blanking.

@6by9: which routines (in what sequence) were called to resume from blanking?

Once the driver is configured there is a procedure to stop/restart video, no need to reconfigure (which most likely fails due to the incorrect DSI status).
1. Host issues a ULPS entry sequence to all DSI CLK and data lanes enabled.
2. When host is ready to exit the ULPS mode, host issues a ULPS exit sequence to all DSI CLK and data lanes that need to be active in normal operation.
3. Wait for the PLL_LOCK bit (CSR 0x0A.7) to be set.
4. Set the SOFT_RESET bit (CSR 0x09.0). 5. Device resumes normal operation.(i.e video streaming resumes on the panel).
Last edited by aBUGSworstnightmare on Fri May 28, 2021 10:24 am, edited 1 time in total.

milo246
Posts: 73
Joined: Mon Mar 01, 2021 1:43 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 9:39 am

aBUGSworstnightmare wrote: it's not related to DSI here. It's the mode defined for the panel (in panel-simple.c) as the DSI link can keep sending 24-bit; no need to send 18bit data only because the bridge will happily convert 24-bit input to 18-bit output. Way simpler than configurimg the mode for in- and output.


Though it'd be neat to negotiate the 18-bit mode to the DSI as well, to save on power if nothing else, the bridge chip driver currently just always asks for 24-bit. But it should still look at the DSI settings in effect, in case the DSI driver has decided differently for whatever reason. And just to be prepared for a future where that negotiation would actually work.

It's only one line of code actually...

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 10:08 am

milo246 wrote:
Fri May 28, 2021 9:39 am
It's only one line of code actually...
Which one and where to add?

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 11246
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 3:42 pm

aBUGSworstnightmare wrote:
Fri May 28, 2021 4:51 am
hi 6by9, thanks for your support/feedback. Let me try it today and report back.
Edit1: pulled the repro, changed DSI lane config in overlay to 3 (4-lane didn't work so far, so will test later; 3 is a save start), enabled the bridge and PWM via menuconfig and started compiling...

Edit2: instantly worked after kernel compile/install with the display mode defined as JEIDA-24 (MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA; that's what the current setting in panel-simple are).
That's good news!
I'm not sure why it's suddenly happy with getting the config messages during enable which is after the encoder is enabled so DSI data lanes are HS.

What resolution are you working with? 4 lanes is working with the JDI LT070ME05000 (Nexus 7 2013) panel at 1200x1920. There are funny rules for where the number of bytes per line isn't an integer multiple of the number of lanes, but that is actually a moderately rare condition (1366 probably being the awkward one again)
aBUGSworstnightmare wrote:
Fri May 28, 2021 9:32 am
Driver still fails on screen blanking.

@6by9: which routines (in what sequence) were called to resume from blanking?

Once the driver is configured there is a procedure to stop/restart video, no need to reconfigure (which most likely fails due to the incorrect DSI status).
1. Host issues a ULPS entry sequence to all DSI CLK and data lanes enabled.
2. When host is ready to exit the ULPS mode, host issues a ULPS exit sequence to all DSI CLK and data lanes that need to be active in normal operation.
3. Wait for the PLL_LOCK bit (CSR 0x0A.7) to be set.
4. Set the SOFT_RESET bit (CSR 0x09.0). 5. Device resumes normal operation.(i.e video streaming resumes on the panel).
disable_outputs.
It should be
- drm_atomic_bridge_chain_disable - bridge/panel disables, starting at the one furthest from the encoder
- encoder disable
- drm_atomic_bridge_chain_post_disable - bridge/panel post_disables starting at the one closest to the encoder.

sn65dsi83_post_disable puts the SN65DSI83 in reset anyway, so I wouldn't expect any state to persist as long as that line is actually wired up correctly.
I don't believe there is a mode within DRM that puts devices into a warm standby type state.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

aBUGSworstnightmare
Posts: 2934
Joined: Tue Jun 30, 2015 1:35 pm

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 6:57 pm

6by9 wrote:
Fri May 28, 2021 3:42 pm
That's good news!
I'm not sure why it's suddenly happy with getting the config messages during enable which is after the encoder is enabled so DSI data lanes are HS.

What resolution are you working with? 4 lanes is working with the JDI LT070ME05000 (Nexus 7 2013) panel at 1200x1920. There are funny rules for where the number of bytes per line isn't an integer multiple of the number of lanes, but that is actually a moderately rare condition (1366 probably being the awkward one again)
the displays which I've tested with so far have 800x1280pixels, the other one 1280x800. None of them have worked on 4 lanes so far. Yes, I know the JDI is working with 4 lanes, that's why it's weird (1280 and 800 are divideable by 8 as integer ).
6by9 wrote:
Fri May 28, 2021 3:42 pm
aBUGSworstnightmare wrote:
Fri May 28, 2021 9:32 am
Driver still fails on screen blanking.

@6by9: which routines (in what sequence) were called to resume from blanking?

Once the driver is configured there is a procedure to stop/restart video, no need to reconfigure (which most likely fails due to the incorrect DSI status).
1. Host issues a ULPS entry sequence to all DSI CLK and data lanes enabled.
2. When host is ready to exit the ULPS mode, host issues a ULPS exit sequence to all DSI CLK and data lanes that need to be active in normal operation.
3. Wait for the PLL_LOCK bit (CSR 0x0A.7) to be set.
4. Set the SOFT_RESET bit (CSR 0x09.0). 5. Device resumes normal operation.(i.e video streaming resumes on the panel).
disable_outputs.
It should be
- drm_atomic_bridge_chain_disable - bridge/panel disables, starting at the one furthest from the encoder
- encoder disable
- drm_atomic_bridge_chain_post_disable - bridge/panel post_disables starting at the one closest to the encoder.

sn65dsi83_post_disable puts the SN65DSI83 in reset anyway, so I wouldn't expect any state to persist as long as that line is actually wired up correctly.
I don't believe there is a mode within DRM that puts devices into a warm standby type state.
will have to thinka about this.
Yes, post_disable de-asserts Enable signal which sends the device to reset (all CSR will need to be reconfigured), but what makes the re-initialization after screen blanking different from the power-on (first during OS boot) initialization?

Smndk
Posts: 13
Joined: Fri Apr 30, 2021 11:24 pm
Location: Denmark

Re: Rpi 4 with DRM and 7inch panel using kms driver

Fri May 28, 2021 11:31 pm

Hi

I am trying to get Android 11 with the 5.10 kernel and full KMS working with HDMI and the official RPI display on a RPI4. I know that this thread isn't related to Android, and now also are discussing other displays, but I hope it is ok that I post my questions here.

I have been building the 5.10 kernel using the https://github.com/android-rpi/kernel_manifest repository . And are using it with my build of Android 11. I am using the vc4-kms-v3d-rpi4 and vc4-kms-dsi-7inch overlays. I works very well with HDMI, but I have a hard time getting it to work with the official RPI display.

What I have found out so far is:
1) The i2c_mux and i2c_mux_pinctrl drivers were not available and the lcd@45 device were not probed. I have fixed that.
2) The i2c driver are being initialized (bind) after the first attempt to initialize the vc4 driver. In this case several attempts are being made to initialize the vc4 driver, and the i2c driver has been initialized at the second attempt. This results in the RPI panel being found and attempted initialized. But no graphics works (with Android anyway) if the vc4 driver isn't initialized successfully at the first attempt. Is it normal that the i2c driver isn't initialized before the vc4 driver? Can initialization of the two be swapped?
3) I can see that some work are being done by 6by9 on the drivers in his own branch of the kernel. But I have been told that it should be possible to make the official display work using the mainline of the 5.10 kernel. Is that correct?

Kind regards,
Svend

Return to “Interfacing (DSI, CSI, I2C, etc.)”