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

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

Sat May 15, 2021 6:37 am

It doesn't state that in the datasheet, the division by 2 of the clock, at least not at the register description.

1. The Marek driver indeed isn't working yet as stated it has the missing mode calls. For now I hacked both calls into the pre-enable to at least get it to work.

2. The ones that are not correct for our panel are the voltage swing

Code: Select all

// 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
// set reverse channel A LVDS, TODO: configure through DTS
/*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);
The horizontal front porch is also 2 pixels off, which I don't care about at the moment and will figure out. A screen should appear nonetheless I would think.

3. I found that formula in the explanation of the DSI Tuner 2 tool: https://e2e.ti.com/support/interface-gr ... annel-lvds
Although I've seen that formula at other places too.

4. We've got a 1920x1080 panel, the Innolux G156HCE-L01

5. Ehm, isn't the Milo246 driver the one with the 4 files, where the bridge functionality is splitted into 4 separate files? No, I personally didn't and that one even has some hardcoded stuff in it that I need to modify.

6. Hmm, you're right. This one also states that: https://www.ti.com/lit/an/slla356/slla3 ... 65DSI83-Q1
However, our hardware design company came up with the following explanation which is exactly what the found formula outputs:
The used panel requires an LVDS pixelclock of min 60MHz (typical 70,93MHz, max 75MHz)
During 1 LVDS pixelclock period, the LVDS side transfers 7 bits per lane.
We've got 2x4 lanes, so in total 8x7 bits, of which 48 bits is rgb data.
Per DSI clock period, a single DSI lane transfers 2 bits.
We've got 4 DSI lanes, so in total 4x2 bits.
Roughly we need a DSI clock of 48/8 x LVDS clock = min 360MHz (typical 425,58MHz, max 450MHz)
7. Agreed

I've had the test pattern running before with hardcoded register values (although with the Milo driver I think and strangely with a much lower DSI clock) and am now attempting to achieve the same through the bridge enable method in the Marek driver as that has a lot of traction.

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

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

Sat May 15, 2021 7:42 am

1. The Marek driver indeed isn't working yet as stated it has the missing mode calls. For now I hacked both calls into the pre-enable to at least get it to work
can you post your changes here So I can test as well?

This is the milo246 driver which is working for me (with a few minor changes as described in the later posts): viewtopic.php?f=44&t=305690&start=125#p1861683

Incorrect voltage swing and termination should not be a show stopper once correctly initialized.

Checked the G156HCE-L01 spec: page 17/31, 4.5
As you can see the LVDS clock of typ 70.93MHz is for one channel only. Note that Thd for this clock is 960pixels, hence it is devided already.

Pls note that I could not get the DSI84 running on 4-lane DSI input so far with two WXGA panels, only 3-lane max.

the more DSI lanes the lower your DSI clock is.

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

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

Sat May 15, 2021 8:47 am

As it seems, the DSI clock depends on the single or dual DSI input selection:

https://www.youtube.com/watch?v=X1svE1d7t6A

So for single DSI input, it needs my formula with 2 *, for dual DSI input it needs the formula as in Marek's driver (so without the * 2).

The LVDS clock range has no relation to single or dual output as shown in the video, there is no correction on the LVDS clock...

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

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

Sat May 15, 2021 9:10 am

UberDoefus wrote:
Sat May 15, 2021 8:47 am
As it seems, the DSI clock depends on the single or dual DSI input selection:

https://www.youtube.com/watch?v=X1svE1d7t6A

So for single DSI input, it needs my formula with 2 *, for dual DSI input it needs the formula as in Marek's driver (so without the * 2).

The LVDS clock range has no relation to single or dual output as shown in the video, there is no correction on the LVDS clock...
sorry, but I think your understanding is wrong! Dual DSI is only possible with DSI85 and dual DSI with dual channel LVDS is a special use case (left/right or odd/even) then.

Dual channel LVDS from single DSI input is possible with DSI84/85. . Have you checked the video which explains the LVDS clock settings? Depending on which timing is shown in the spec one need to input only half of the values for a horizontal line. Again, look at your spec: LVDS clock for a line (1920pixels) is 2x70.93MHz , so you can have a. Spec which shows 141.86MHz for 1920 pixels --> such value needs to be devided by 2 for the DSI8x devices.

And for sure DSI clock depends on LVDs clock! Dual channel LVDS has lower EMI emmisions because it has two lower frequency clocks (compared to using one clock only).
Same is valid for DSI; clock can be reduced if LVDS clock is lower/no of channels bigger.

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

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

Sat May 15, 2021 9:41 am

aBUGSworstnightmare wrote:
Sat May 15, 2021 9:10 am
UberDoefus wrote:
Sat May 15, 2021 8:47 am
As it seems, the DSI clock depends on the single or dual DSI input selection:

https://www.youtube.com/watch?v=X1svE1d7t6A

So for single DSI input, it needs my formula with 2 *, for dual DSI input it needs the formula as in Marek's driver (so without the * 2).

The LVDS clock range has no relation to single or dual output as shown in the video, there is no correction on the LVDS clock...
sorry, but I think your understanding is wrong! Dual DSI is only possible with DSI85 and dual DSI with dual channel LVDS is a special use case (left/right or odd/even) then.

Dual channel LVDS from single DSI input is possible with DSI84/85. . Have you checked the video which explains the LVDS clock settings? Depending on which timing is shown in the spec one need to input only half of the values for a horizontal line. Again, look at your spec: LVDS clock for a line (1920pixels) is 2x70.93MHz , so you can have a. Spec which shows 141.86MHz for 1920 pixels --> such value needs to be devided by 2 for the DSI8x devices.

And for sure DSI clock depends on LVDs clock! Dual channel LVDS has lower EMI emmisions because it has two lower frequency clocks (compared to using one clock only).
Same is valid for DSI; clock can be reduced if LVDS clock is lower/no of channels bigger.
Alright, so in my DTS, I need to multiply everything by 2, not only the clock but also the h-parts?

Code: Select all

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 */
	clock-frequency = <141860000>;

	/* the hactive and porches in the datasheet are for a single LVDS channel, describe the complete panel here */
	hactive = <1920>;
	hsync-len = <70>;
	hfront-porch = <70>;
	hback-porch = <70>;

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

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

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

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

Sat May 15, 2021 12:06 pm

Bildschirmfoto 2021-05-15 um 13.57.53.png
timing details from G156HCE-L01
Bildschirmfoto 2021-05-15 um 13.57.53.png (194.34 KiB) Viewed 1323 times
It is important to note that all of the horizontal timing parameters have been divided by 2. For example, your datasheet shows that the number of active horizontal pixels is 960 --> this is because the timing information is for one channel or one half of your display.
Think of a display vertically cut in half into two separate displays – the amount of vertical lines stays the same but all of the horizontal parameters (frequency, active pixels, blanking pixels) will also be cut in half. (Note: that is something which I can see on the Nexus Gen7 display in case it's not driven correctly. Let me try to take a picture and it's more easy to understand what's going on).
In case your display spec does not show the horizontal parameters divided by 2 (you can easily tell by seeing whether or not the horizontal active pixels have been halved; explained that already), then you will need to ensure to divide them by 2 by yourself.
If your display datasheet only shows the total blanking and not HPW, HBP, HFP, VPW, VBP, and VFP individually, then you simply need to make sure that the sum of HPW, HBP, and HFP add up to the total horizontal blanking, and that VPW, VBP, and VFP add up to the total vertical blanking.

So, for the driver to work you need to multiply them by two.

You have this timing details added to your DT overlay, have you? Looks o.k. on first view.
Neverthless let me offer again to test Mareks driver as well, by adding the changes you made and using your overlay template.

Overlay that I'm using has been posted here already. My timing details were added to 'panel-simple.c' and not part of the DT overlay.

Edit: just adding some reference here: https://elixir.bootlin.com/linux/v5.12/ ... des.h#L173

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

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

Sat May 15, 2021 7:02 pm

No problem, below you'll find the dts and the modified bridge driver. Today, with hardcoded settings for the registers, I have the testpattern running on a DSI clock of 375Mhz. Next attempt is to dump some random pixels onto the fb0.

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 6 */
					/* (3Ghz / 3)/6 = 166.666Mhz. (166.666Mhz * 24bpp) / 2*#lanes = 500Mhz DSI clock */
					/* (3Ghz / 4)/6 = 125Mhz. (125Mhz * 24bpp) / 2*#lanes = 375Mhz DSI clock */
					/* we'll choose for the 375Mhz DSI clock, with a divider of 6 in the bridge, resulting in 62.5Mhz */
					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 3>;
						};
					};

					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 3>;
				};
			};
		};
	};

	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";
		};
	};
};
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, 0x28},//ok
	{0x0D, 0x00},//ok
	{0x10, 0x26},//ok
	{0x11, 0x00},//ok
	{0x12, 0x4B},//ok
	{0x13, 0x00},//ok
	{0x18, 0x6C},//ok
	{0x19, 0x0F},//ok
	{0x1A, 0x20},//ok
	{0x1B, 0x00},//ok

	/* Channel A */
	{0x20, 0xC0},//ok
	{0x21, 0x03},//ok
	{0x24, 0x38},//ok
	{0x25, 0x04},//ok
	{0x28, 0x21},//ok
	{0x29, 0x00},//ok
	{0x2C, 0x1E},//ok
	{0x2D, 0x00},//ok
	{0x30, 0x08},//ok
	{0x31, 0x00},//ok
	{0x34, 0x1E},//ok
	{0x36, 0x06},//ok
	{0x38, 0x1E},//ok
	{0x3A, 0x06},//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},
#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);

#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;

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

	/* 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
	// set reverse channel A LVDS, TODO: configure through DTS
	/*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
	regmap_bulk_write(ctx->regmap, REG_VID_CHA_ACTIVE_LINE_LENGTH_LOW,
			&ctx->mode.hdisplay, 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;
	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;
	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;
	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
	regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x00);
	//regmap_write(ctx->regmap, REG_VID_CHA_TEST_PATTERN, 0x10);

	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");
The only doubt I'm having are all the porches, the dsi-tuner divides them by 2 whereas the Marek driver uses them as is.

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

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

Sun May 16, 2021 6:07 am

Please allow one question: why do you have 'compatible = "innolux,g156hce-l01", "panel-lvds";' in your overlay when all timings are defined in the overlay as well?

Will change hardcoded register settings and then test.
I still think there are issues with the driver as REG_RC_RESET needs to be set after changes to CSRs have been made, but it accessed upfront. In addition I think that 'jeida-18' will fail.

I also don't understand why they use 'u32 input_bus_format = MEDIA_BUS_FMT_RGB888_1X24' in 'sn65dsi83_mode_set' when the input format specifier is 'MIPI_DSI_FMT_RGB888'

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

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

Sun May 16, 2021 6:45 am

aBUGSworstnightmare wrote:
Sun May 16, 2021 6:07 am
Please allow one question: why do you have 'compatible = "innolux,g156hce-l01", "panel-lvds";' in your overlay when all timings are defined in the overlay as well?

Will change hardcoded register settings and then test.
I still think there are issues with the driver as REG_RC_RESET needs to be set after changes to CSRs have been made, but it accessed upfront. In addition I think that 'jeida-18' will fail.

I also don't understand why they use 'u32 input_bus_format = MEDIA_BUS_FMT_RGB888_1X24' in 'sn65dsi83_mode_set' when the input format specifier is 'MIPI_DSI_FMT_RGB888'
The 'compatible = "innolux,g156hce-l01", "panel-lvds";' is there to indicate the yaml that is used and what driver is used afaik. The innolux,g... is superfluous but doesn't harm either, it's a relic because I used the simple-panel first. I've got no timings in any panel source file, only in the DTS.

Accessing the REG_RC_RESET doesn't harm before the changes have been done, as long as it is also accessed after the changes have been done, which it is. In my hack, I disregard jeida because my panel is vesa, so if your addressing that, you're right.

I don't understand your remark about the input format, sn65dsi83_mode_set accepts the MEDIA_BUS_FMT_RGB888_1X24 as format, so could you elaborate more about this?

Note: I indeed had to correct porches for lvds_dual_link, which allowed me using the test-pattern without the hardcoded settings.

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

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

Sun May 16, 2021 7:40 am

UberDoefus wrote:
Sun May 16, 2021 6:45 am
Accessing the REG_RC_RESET doesn't harm before the changes have been done, as long as it is also accessed after the changes have been done, which it is.

Code: Select all

static void sn65dsi83_enable(struct drm_bridge *bridge)
{..
	/* Clear reset, disable PLL */
	regmap_write(ctx->regmap, REG_RC_RESET, 0x00);
	regmap_write(ctx->regmap, REG_RC_PLL_EN, 0x00);
..
The above is either to be removed or changed to REG_RC_PLL_EN accessed first, followed by REG_RC_RESET because SOFT_RESET bit must be set after the CSRs are updated. Actually it is done before, so stopping the PLL (PLL_EN=0) will not be executed with this sequence.
UberDoefus wrote:
Sun May 16, 2021 6:45 am
The 'compatible = "innolux,g156hce-l01", "panel-lvds";' is there to indicate the yaml that is used and what driver is used afaik. The innolux,g... is superfluous but doesn't harm either, it's a relic because I used the simple-panel first. I've got no timings in any panel source file, only in the DTS.
understood, so this line get's ignored.
UberDoefus wrote:
Sun May 16, 2021 6:45 am
I don't understand your remark about the input format, sn65dsi83_mode_set accepts the MEDIA_BUS_FMT_RGB888_1X24 as format, so could you elaborate more about this?
I don't understand why a different mode - which is for DPI - get's used as the data that is input over DSI is according to MIPI_DSI_FMT_RGB888 --> always 24-bit. The bridge driver then should do the initialization if 24-bit to 18-bit data conversion is required.
Will test this once I've got the driver working.
UberDoefus wrote:
Sun May 16, 2021 6:45 am
Note: I indeed had to correct porches for lvds_dual_link, which allowed me using the test-pattern without the hardcoded settings.
You have more than test pattern already?

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

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

Sun May 16, 2021 10:24 am

The above is either to be removed or changed to REG_RC_PLL_EN accessed first, followed by REG_RC_RESET because SOFT_RESET bit must be set after the CSRs are updated. Actually it is done before, so stopping the PLL (PLL_EN=0) will not be executed with this sequence.
Correct, I switched both lines. I guess a good spot for disabling the PLL would be the pre-enable.

No, I'm not getting anything yet when using the dev/fb0. Fbset also outputs something unexpected:

Code: Select all

root@raspberrypi4-64:~# fbset

mode "1920x1080-0"
	#D: 0.000Mhz, H: 0.000kHz, V: 0.000kHz
	geometry 1920 1080 1920 1080 16
	timings 0 0 0 0 0 0 0
	accel true
	rgba 5/11,6/5,5/0,0/0
endmode
16bpp isn't correct, frequencies and timings aren't known

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

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

Sun May 16, 2021 12:24 pm

UberDoefus wrote:
Sun May 16, 2021 10:24 am
The above is either to be removed or changed to REG_RC_PLL_EN accessed first, followed by REG_RC_RESET because SOFT_RESET bit must be set after the CSRs are updated. Actually it is done before, so stopping the PLL (PLL_EN=0) will not be executed with this sequence.
Correct, I switched both lines. I guess a good spot for disabling the PLL would be the pre-enable.

No, I'm not getting anything yet when using the dev/fb0. Fbset also outputs something unexpected:

Code: Select all

root@raspberrypi4-64:~# fbset

mode "1920x1080-0"
	#D: 0.000Mhz, H: 0.000kHz, V: 0.000kHz
	geometry 1920 1080 1920 1080 16
	timings 0 0 0 0 0 0 0
	accel true
	rgba 5/11,6/5,5/0,0/0
endmode
16bpp isn't correct, frequencies and timings aren't known
see here viewtopic.php?f=44&t=305690&start=125#p1861715 for what to change for 24bpp.
What does 'xrandr --verbose' report? You are working with 64bit kernel btw?
Finished kernel compiling with based on 6by9s sources, patched to Marek V4. Will give it a try again and the change to your hacked version + overlay.

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

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

Sun May 16, 2021 3:44 pm

Nope, bootarg didn't fix 16 bpp into 24 bpp, nor did 'framebuffer_depth=24' in the config.txt.
It appears that having hw_accel enabled restricts me from anything but 16bpp?
https://www.reddit.com/r/raspberry_pi/c ... _can_only/

Yes, I'm running a 64bit kernel and xrandr doesn't work as I have no X installed, so probably giving me the 'Can't open display' because of it.

Maybe I need to go for a 3 lane setup iso 4 lanes as you guys have already mentioned that...

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

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

Sun May 16, 2021 4:42 pm

UberDoefus wrote:
Sun May 16, 2021 3:44 pm
Nope, bootarg didn't fix 16 bpp into 24 bpp, nor did 'framebuffer_depth=24' in the config.txt.
It appears that having hw_accel enabled restricts me from anything but 16bpp?
https://www.reddit.com/r/raspberry_pi/c ... _can_only/

Yes, I'm running a 64bit kernel and xrandr doesn't work as I have no X installed, so probably giving me the 'Can't open display' because of it.

Maybe I need to go for a 3 lane setup iso 4 lanes as you guys have already mentioned that...
https://github.com/raspberrypi/linux/co ... 648927cd47
Rwvert the above change in your vc4_drv.c
That's what you should find from that link.

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

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

Sun May 16, 2021 7:24 pm

Made that change, indeed I have 32 bits now.
Still nothing on the display, then made the switch to 3 DSI data-lanes (pushing the DSI clock to its max, display running at min LVDS frequency). Testpattern OK, catting to fb0 after disabling the testpattern results in nothing on the display.

Continue tomorrow, maybe jumping to the milo driver as aBugsWorstNightmare has this running? I'm wondering why you're trying the Marek driver?

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

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

Sun May 16, 2021 8:08 pm

UberDoefus wrote:
Sun May 16, 2021 7:24 pm
Made that change, indeed I have 32 bits now.
Still nothing on the display, then made the switch to 3 DSI data-lanes (pushing the DSI clock to its max, display running at min LVDS frequency). Testpattern OK, catting to fb0 after disabling the testpattern results in nothing on the display.

Continue tomorrow, maybe jumping to the milo driver as aBugsWorstNightmare has this running? I'm wondering why you're trying the Marek driver?
not clear which one will be added to upstream, but Mareks most likely.

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

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

Sun May 16, 2021 9:06 pm

aBUGSworstnightmare wrote:
Sun May 16, 2021 8:08 pm
UberDoefus wrote:
Sun May 16, 2021 7:24 pm
Made that change, indeed I have 32 bits now.
Still nothing on the display, then made the switch to 3 DSI data-lanes (pushing the DSI clock to its max, display running at min LVDS frequency). Testpattern OK, catting to fb0 after disabling the testpattern results in nothing on the display.

Continue tomorrow, maybe jumping to the milo driver as aBugsWorstNightmare has this running? I'm wondering why you're trying the Marek driver?
not clear which one will be added to upstream, but Mareks most likely.
So to get this straight, we have 2 (or more) drivers that might be added to upstream and the tendency is to integrate a driver which you can't get to work over a driver that you did got to work? Am I the only one not comprehending this? At this point in time, my customer and employer don't care which driver works, as long as we can get it to work. If that means using a forked kernel with a custom driver then so be it. We're even debating internally if we need to buy or rent a differential 1Ghz scope to get this thing running...

Did anyone look at the differences between both drivers? Can't be that big of a difference since they both address the same bridge IC.

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

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

Sun May 16, 2021 9:10 pm

Bingo, hit the jackpot. Noticed 1 wrong register value and catting /dev/random to /dev/fb0 works! :D On a 3 lane setup that is, but will try a 4 lane setup now.

Edit: I can confirm, 4 lane setup doesn't work.

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

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

Mon May 17, 2021 4:57 am

UberDoefus wrote:
Sun May 16, 2021 9:10 pm
Bingo, hit the jackpot. Noticed 1 wrong register value and catting /dev/random to /dev/fb0 works! :D On a 3 lane setup that is, but will try a 4 lane setup now.

Edit: I can confirm, 4 lane setup doesn't work.
would be good ro state which register and if you got it running with milo246 or Marek V4 driver (either hacked or in it's original state as submitted to patchwork )
JDI FHD MIPI panel (Google Nexus 7 Gen2) is able to run on 4 lanes, so it's either the bridge which struggles here or some (still not perfect) clock divider settings which hinder the bridge from using all available lanes.

Having a driver in upstream would be preferred, so users don't need to compile a custom kernel.

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 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?

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

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

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

Mon May 17, 2021 6:52 am

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?

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
6by9 (oe another RPI engineer) need to answer to this.

Should be best to contact Marek directly. They are implementing the driver on i.MX8 and will have to review your changes.

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 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.

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

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

Mon May 17, 2021 8:26 am

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.
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.

@milo246: how do you adjusf this atm? Starting point needs to be LVDS clock as a function of achievable/reasonable DSI clock.

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 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.

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 8:56 am

Milo246,

you captured exactly how I adressed the clocks and display this weekend. I just calculated backwards from the DSI clock to the panel and not vice-versa.
Thanks for confirming my thoughts.

How I calculated it:
Internally, the vc4_dsi.c starts iterating over the possible dividers (x) to search for a result that matches or exceeds the timing in the DTS

3Ghz / x / (24bpp/#nr-lanes) = y; y * 24bpp / 8 = DSI clock;

So for an example:

With 3 data-lanes I need a minimal DSI clock of 48bits/2*3 * 60Mhz(min) = 480Mhz to be able to transfer all data. So I'm aiming for a DSI clock that is at least that. The max DSI clock is 500Mhz so I cannot exceed that.

3/1/(24/3) = 428.57Mhz; 428.57 * 24 / 8 = 1.7Ghz which is an invalid DSI clock, will be capped to 500Mhz. 428.57Mhz is an invalid dual LVDS frequency.
3/2/(24/3) = ...
3/3/(24/3) = 142.86Mhz; 142.86 * 24 / 8 = 571Mhz which is an invalid DSI clock, will be capped to 500Mhz. 142.86Mhz could be a valid dual LVDS frequency.
3/4/(24/3) = 107.14Mhz; 107.14 * 24 / 8 = 321Mhz which is a valid DSI clock but not enough to meet the 480Mhz requirement.

So the vc4_dsi driver picks the divider or 3 and caps the DSI clock to 500Mhz, this is > 480Mhz so is ok. Based on that decision of 500Mhz, I check what divider I need in the bridge: 500Mhz/bridge divider of 8 == 125Mhz. 125Mhz for a dual LVDS means 62.5Mhz per channel which is just above the minimum of the 60Mhz. So this 125Mhz is a valid frequency to be used in the DTS.

This means that I should set the DTS panel timing to 125Mhz to match the DSI clock. This 125Mhz indeed falls between the 107.14 and 142.86Mhz.
Last edited by UberDoefus on Mon May 17, 2021 9:23 am, edited 4 times in total.

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