panel-simple-dsi driver was added in 2022 but looks like a duplicate and code is buggy - removal?

prahal · July 18, 2023

I found what a likely duplicate out-of-tree driver after investigating a warning from the kernel build:

   CC [M]  drivers/gpu/drm/panel/panel-simple-dsi.o                                                                                                                                                               
  drivers/gpu/drm/panel/panel-simple-dsi.c: In function 'panel_simple_get_modes':                                                                                                                                  
  drivers/gpu/drm/panel/panel-simple-dsi.c:310:73: warning: passing argument 3 of 'of_get_drm_display_mode' makes pointer from integer without a cast [-Wint-conversion]                                           
    310 |         ret = of_get_drm_display_mode(panel->dev->of_node, mode, p->desc->bus_format,                                                                                                                    
        |                                                                  ~~~~~~~^~~~~~~~of_get_drm_display_mode~~~~                                                                                              
        |                                                                         |                                                                                                                                
        |                                                                         u32 {aka unsigned int}                                                                                                           
  In file included from ./include/drm/drm_crtc.h:32,                                                                                                                                                               
                   from drivers/gpu/drm/panel/panel-simple-dsi.c:13:                                                                                                                                               
  ./include/drm/drm_modes.h:509:66: note: expected 'u32 *' {aka 'unsigned int *'} but argument is of type 'u32' {aka 'unsigned int'}                                                                               
    509 |                             struct drm_display_mode *dmode, u32 *bus_flags,                                                                                                                              
        |                                                             ~~~~~^~~~~~~~~

This code cannot work as is and a fix would be to add the missing ampersand before the p->desc->bus_format. Still other warning surface when this is fixed.

This driver calls of_get_drm_display_mode against bus_format while panel-simple does against bus_flags (it also has bus_format but it passes it as an argument to drm_display_info_set_bus_formats).

So it might well be that the driver would pass the wrong arguments.

It has no documentation for its DTS parameters so it is hard to tell if bus_format should be set as bus_flags from the panel-simple documentation or if its user will end up sending bus_format as a bus_flags.

There is already a dsi driver in upstream drivers/gpu/drm/panel/panel-simple.c (panel-dsi-simple while this one is panel-simple-dsi).

So I wonder if there is a use in shipping two drivers for the same purpose.

And there are no dts users of this out-of-tree driver panel-simple-dsi.

The discussion about its introduction is in: https://github.com/armbian/build/pull/3140

It is told in the commit log to be a port of a rockchip kernel 4.4 driver, but panel-simple has dsi probe since v3.14. I am puzzled rockchip forked panel-simple instead of expanding it.

Rockchip.DRM.Panel.Porting.Guide.pdf has no mention of the bus_format dts parameter so maybe this code path was never used and the buggy code affects no users.

(a few dts parameters in the rockchip code are also not documented in this PDF).

@iamdrq you are the author of this driver. Was there code missing in the dis support of panel-simple.c for dsi support, ie is panel-simple-dsi necessary?

Was the aim to have a version of the panel dsi code that is bit-to-bit compatible with the rockchip kernel, so no porting was necessary?

If features were missing could we bake them into the panel-simple.c driver (so they could be tested and one day be upstreamed)?

iamdrq · July 18, 2023

I'm sorry about this driver has 4 warnings when I checked it,and the bus_format actually is bus_flags, I used the wrong name, although it not effect use, I will fix it when I have time in the future.

This dirver is differ in upstream panel-simple-dsi, panel-simple-dsi actually contain a lot sets panel model.but this panel-dsi-simple is universal driver,you just put init code,config to dts and not need adjsut driver c code when drive a new panel model.

prahal · July 18, 2023

@iamdrqthanks for the feedback.

So it is a "panel dsi code that is bit-to-bit compatible with the rockchip kernel" which is not aimed at ever being upstreamed?

But isn't it opening the door to diverging from upstream definitely?

I mean if the board definitions start to rely on such drivers that are not even proposed upstream because it is against their approved design, these boards will be harder and harder to upstream.

I do not know Armbian policy. I am just used to Debian whose policy is to only accept patches to adapt programs to the distribution and patches that could eventually end up upstreamed.

I am interested in understanding the policy of Armbian regarding such eternal divergences.

If such patches are deemed good, then after replacing p->desc->bus_format by &p->desc->bus_format (well really bus_flags , though then the OF bus-format property should be renamed too to avoid confusion), a warning triggers:

 drivers/gpu/drm/panel/panel-simple-dsi.c: In function 'panel_simple_get_modes':                                                                                                                                   
 drivers/gpu/drm/panel/panel-simple-dsi.c:310:66: warning: passing argument 3 of 'of_get_drm_display_mode' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]                            
   310 |         ret = of_get_drm_display_mode(panel->dev->of_node, mode, &p->desc->bus_format,                                                                                                                    
       |                                                                  ^~~~~~~~~~~~~~~~~~~~                                                                                                                     
 In file included from ./include/drm/drm_crtc.h:32,                                                                                                                                                                
                  from drivers/gpu/drm/panel/panel-simple-dsi.c:13:                                                                                                                                                
 ./include/drm/drm_modes.h:509:66: note: expected 'u32 *' {aka 'unsigned int *'} but argument is of type 'const u32 *' {aka 'const unsigned int *'}
   509 |                             struct drm_display_mode *dmode, u32 *bus_flags,                     
       |                                                             ~~~~~^~~~~~~~~

Edited July 18, 2023 by prahal

iamdrq · July 18, 2023

Full remove warning :

ret = of_get_drm_display_mode(panel->dev->of_node, mode, p->desc->bus_format,
change to:
ret = of_get_drm_display_mode(panel->dev->of_node, mode, (u32 *)&p->desc->bus_format,

and delete unused variable line 379,470,525

I am not familiar upstream policy and it aim is not bit-to-bit compatible with the rockchip kernel.

This patch driver make drive a dsi panel easy, so I shared this to armbian build system.I hope it help someone drive dsi panel easy.

Jack Huang · August 21

@iamdrq hello bro, have you test mipi dsi on nanopc-t4 kernel v6.1? I use this patch apply in friendlyarm offical kernel v6.1 https://github.com/friendlyarm/kernel-rockchip, but no display and backlight is ok, display well in friendlyarm offical kernel v4.19. Could you help me test mipi dsi on nanopc-t4 in armbian with linux v6.1? My linux v6.1.y repo is https://github.com/JackHuang021/kernel-nanopct4

// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
 * FriendlyElec NanoPC-T4 board mipi dis device tree source
 */

#include <dt-bindings/display/drm_mipi_dsi.h>

/ {
	backlight: backlight {
		compatible = "pwm-backlight";
		pwms = <&pwm0 0 25000 0>;
		brightness-levels = <
			  0 20 20 20 20 20 20 20
			  21 21 21 21 21 21 21 21
			  22 22 22 22 22 22 22 22
			  23 23 23 23 23 23 23 23
			  24 24 24 24 24 24 24 24
			  25 25 25 25 25 25 25 25
			  26 26 26 26 26 26 26 26
			  27 27 27 27 27 27 27 27
			  28 28 28 28 28 28 28 28
			  29 29 29 29 29 29 29 29
			  30 30 30 30 30 30 30 30
			  31 31 31 31 31 31 31 31
			  32 32 32 32 32 32 32 32
			  33 33 33 33 33 33 33 33
			  34 34 34 34 34 34 34 34
			  35 35 35 35 35 35 35 35
			  36 36 36 36 36 36 36 36
			  37 37 37 37 37 37 37 37
			  38 38 38 38 38 38 38 38
			  39 39 39 39 39 39 39 39
			  40 40 40 40 40 40 40 40
			  41 41 41 41 41 41 41 41
			  42 42 42 42 42 42 42 42
			  43 43 43 43 43 43 43 43
			  44 44 44 44 44 44 44 44
			  45 45 45 45 45 45 45 45
			  46 46 46 46 46 46 46 46
			  47 47 47 47 47 47 47 47
			  48 48 48 48 48 48 48 48
			  49 49 49 49 49 49 49 49
			  50 50 50 50 50 50 50 50
			  51 51 51 51 51 51 51 51
			  256
		>;
		default-brightness-level = <50>;
	};
};

&i2c4 {
	status = "okay";
	ts@5d {
		pinctrl-0 = <&touch_int_gpio>;
		compatible = "goodix,gt9xx";
		reg = <0x5d>;
		tp-size = <911>;
		max-x = <720>;
		max-y = <1280>;
		touch-gpio = <&gpio1 RK_PC4 IRQ_TYPE_LEVEL_LOW>;
		reset-gpio = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>;
	};
};

&mipi_in_vopl {
	status = "okay";
};

&mipi_in_vopb {
	status = "disabled";
};

&hdmi_in_vopb {
	status = "okay";
};

&hdmi_in_vopl {
	status = "disabled";
};

&mipi_dsi {
	status = "okay";
	rockchip,lane-rate = <580>; //(720+8+8+16)*(1280+8+8+16)*60*3*8/3,+100mhz
	dsi_panel: panel@0 {
		status = "okay";
		compatible = "panel-dsi-simple";
		reset-gpios = <&gpio4 RK_PD6 GPIO_ACTIVE_LOW>;
		pinctrl-names = "default";
		pinctrl-0 = <&dsi_rst_gpio>;
		reg = <0>;
		backlight = <&backlight>;
		reset-delay-ms = <200>;
		enable-delay-ms = <100>;
		prepare-delay-ms = <20>;
		unprepare-delay-ms = <20>;
		disable-delay-ms = <20>;
		init-delay-ms = <120>;
		dsi,flags = <(MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
			MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_EOT_PACKET)>;
		dsi,format = <MIPI_DSI_FMT_RGB888>;
		dsi,lanes  = <3>;
		width-mm = <74>;
		height-mm = <132>;
		panel-init-sequence = [
			// init code
			// Set EXTC
			39 00 04 B9 FF 83 94
			// Set MIPI
			39 00 03 BA 72 83
			// Set Power HX5186 Mode
			39 00 10 B1 6C 15 15 24 E4 11 F1 80 E4 97 23 80 C0 D2 58
			// Set Display
			39 00 0C B2 00 64 10 07 22 1C 08 08 1C 4D 00
			// Set CYC
			39 00 0D B4 00 FF 03 5A 03 5A 03 5A 01 6A 30 6A
			// Set VDC
			15 00 02 BC 07
			// Set Power ption HX5186 Mode
			39 00 04 BF 41 0E 01
			// Set D3
			39 00 1F D3 00 06 00 40 07 08 00 32 10 07 00 07 54 15 0F 05 04 02 12 10 05 07 33 33 0B 0B 37 10 07 07
			// Set GIP,Forward
			39 00 2D D5 04 05 06 07 00 01 02 03 20 21 22 23 18 18 18 18 18 18 18 18 19 19 18 18 18 18 1B 1B 1A 1A 18 18 18 18 18 18 18 18 18 18 18 18 18 18
			// Set D6,Backward
			39 00 2D D6 03 02 01 00 07 06 05 04 23 22 21 20 18 18 18 18 18 18 58 58 18 18 19 19 18 18 1B 1B 1A 1A 18 18 18 18 18 18 18 18 18 18 18 18 18 18
			// Set Panel
			15 00 02 CC 09 // 08: invert RGB
			// Set VCOM
			39 00 03 B6 51 51
			// Set Gamma
			39 00 2B E0 00 10 16 2D 33 3F 23 3E 07 0B 0D 17 0E 12 14 12 13 06 11 13 18 00 0F 16 2E 33 3F 23 3D 07 0B 0D 18 0F 12 14 12 14 07 11 12 17
			// Set C0
			39 00 03 C0 30 14
			// Set TCON ption
			39 00 05 C7 00 C0 40 C0
			// Set SAP_L ption
			15 00 02 DF 87
			// Set SETOFFSET
			15 00 02 D2 66 
			// 
			15 78 02 11 00
			15 28 02 29 00
		];

		panel-exit-sequence = [
			//05 00 01 28
			//05 00 01 10
		];

		disp_timings: display-timings {
			native-mode = <&dsi_timing0>;
			dsi_timing0: timing0 {
				clock-frequency = <59197440>;// (720+8+8+16)*(1280+8+8+16)*60

				hactive = <720>;
				hfront-porch = <8>;
				hback-porch = <8>;
				hsync-len = <16>;

				vactive = <1280>;
				vfront-porch = <8>;
				vback-porch = <8>;
				vsync-len = <16>;

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

				swap-rb = <0>;
				swap-rg = <0>;
				swap-gb = <0>;
			};
		};

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

			port@0 {
				reg = <0>;
				panel_in_dsi: endpoint {
					remote-endpoint = <&dsi_out_panel>;
				};
			};
		};
	};

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

		mipi_out: port@1 {
			reg = <1>;
			dsi_out_panel: endpoint {
				remote-endpoint = <&panel_in_dsi>;
			};
		};
	};
};

&pinctrl {
	dsi {
		dsi_rst_gpio: dsi-rst-gpio {
			rockchip,pins = <4 RK_PD6 RK_FUNC_GPIO &pcfg_pull_none>;
		};
		touch_int_gpio: touch-int-gpio {
			rockchip,pins = <1 RK_PC4 RK_FUNC_GPIO &pcfg_pull_up>;
		};
	};
};

Jack Huang · August 22

@iamdrq hello bro, have you test mipi dsi on nanopc-t4 kernel v6.1? I use this patch apply in friendlyarm offical kernel v6.1 https://github.com/friendlyarm/kernel-rockchip, but no display and backlight is ok, display well in friendlyarm offical kernel v4.19. Could you help me test mipi dsi on nanopc-t4 in armbian with linux v6.1? My linux v6.1.y repo is https://github.com/JackHuang021/kernel-nanopct4