[PATCH v4 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90

[PATCH v4 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90

[Paul Kocialkowski]

This adds timings for the RGB666 variant of the Innolux AT070TN90 panel,
as found on the Ainol AW1 tablet.

The panel also supports RGB888 output. When RGB666 mode is used instead,
the two extra lanes per component are grounded.

In the future, it might become necessary to introduce a dedicated
device-tree property to specify the bus format to use instead of the
default one for the panel. This will allow supporting different bus
formats for the same panel modes.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
index cbf1ab404ee7..32e30d5a8f08 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1063,6 +1063,29 @@ static const struct panel_desc innolux_at043tn24 = {
 	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
 };
 
+static const struct drm_display_mode innolux_at070tn90_mode = {
+	.clock = 40000,
+	.hdisplay = 800,
+	.hsync_start = 800 + 112,
+	.hsync_end = 800 + 112 + 1,
+	.htotal = 800 + 112 + 1 + 87,
+	.vdisplay = 480,
+	.vsync_start = 480 + 141,
+	.vsync_end = 480 + 141 + 1,
+	.vtotal = 480 + 141 + 1 + 38,
+	.vrefresh = 60,
+};
+
+static const struct panel_desc innolux_at070tn90 = {
+	.modes = &innolux_at070tn90_mode,
+	.num_modes = 1,
+	.size = {
+		.width = 154,
+		.height = 86,
+	},
+	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
+};
+
 static const struct drm_display_mode innolux_at070tn92_mode = {
 	.clock = 33333,
 	.hdisplay = 800,
@@ -2151,6 +2174,9 @@ static const struct of_device_id platform_of_match[] = {
 	}, {
 		.compatible = "innolux,at043tn24",
 		.data = &innolux_at043tn24,
+	}, {
+		.compatible = "innolux,at070tn90",
+		.data = &innolux_at070tn90,
 	}, {
 		.compatible = "innolux,at070tn92",
 		.data = &innolux_at070tn92,
-- 
2.17.0

[PATCH v4 2/3] ARM: dts: sun7i: Add RGB666 pins definition

[Paul Kocialkowski]

This adds the pins definition for RGB666 LCD panels on the A20. It was
inspired by the A33 definition, that concernes the same set of pins.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index e529e4ff2174..b3ee36cd7fa7 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -781,6 +781,14 @@
 				function = "ir1";
 			};
 
+			lcd0_rgb666_pins: lcd0-rgb666 {
+				pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
+				       "PD10", "PD11", "PD12", "PD13", "PD14", "PD15",
+				       "PD18", "PD19", "PD20", "PD21", "PD22", "PD23",
+				       "PD24", "PD25", "PD26", "PD27";
+				function = "lcd0";
+			};
+
 			mmc0_pins_a: mmc0@0 {
 				pins = "PF0", "PF1", "PF2",
 				       "PF3", "PF4", "PF5";
-- 
2.17.0

[PATCH v4 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet

[Paul Kocialkowski]

This adds support for the Ainol AW1, an A20-based 7" tablet from Ainol.

The following board-specific features are supported:
* LCD panel
* Backlight
* USB OTG
* Buttons
* Touchscreen (doesn't work without non-free firmware)
* Accelerometer
* Battery

The following are untested:
* Audio output
* Audio speakers
* USB via SPCI connector

The following are not supported:
* Wi-Fi
* Bluetooth
* NAND
* Audio via SPCI connector
* Audio via Bluetooth I2S

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/Makefile                |   1 +
 arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts | 297 ++++++++++++++++++++++
 2 files changed, 298 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 7e2424957809..4a80971f2bc9 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -946,6 +946,7 @@ dtb-$(CONFIG_MACH_SUN6I) += \
 	sun6i-a31s-sinovoip-bpi-m2.dtb \
 	sun6i-a31s-yones-toptech-bs1078-v2.dtb
 dtb-$(CONFIG_MACH_SUN7I) += \
+	sun7i-a20-ainol-aw1.dtb \
 	sun7i-a20-bananapi.dtb \
 	sun7i-a20-bananapi-m1-plus.dtb \
 	sun7i-a20-bananapro.dtb \
diff --git a/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
new file mode 100644
index 000000000000..9a732e4cd076
--- /dev/null
+++ b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
@@ -0,0 +1,297 @@
+/*
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ *
+ * Copyright (C) 2018 Paul Kocialkowski <contact@paulk.fr>
+ */
+
+/dts-v1/;
+#include "sun7i-a20.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pwm/pwm.h>
+
+/ {
+	model = "Ainol AW1";
+	compatible = "ainol,ainol-aw1", "allwinner,sun7i-a20";
+
+	aliases {
+		serial0 = &uart0;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
+		brightness-levels = <  0   1   1   1   1   2   2   2
+				       2   3   3   3   3   4   4   4
+				       5   5   5   6   6   6   7   7
+				       8   8   8   9   9   9  10  10
+				      10  11  11  12  12  12  13  13
+				      14  14  14  15  15  16  16  17
+				      17  17  18  18  19  19  20  20
+				      21  21  21  22  22  23  23  24
+				      24  25  25  26  26  27  27  28
+				      28  29  30  30  31  31  32  32
+				      33  33  34  35  35  36  36  37
+				      38  38  39  39  40  41  41  42
+				      43  43  44  44  45  46  47  47
+				      48  49  49  50  51  51  52  53
+				      54  54  55  56  57  57  58  59
+				      60  61  61  62  63  64  65  65
+				      66  67  68  69  70  71  71  72
+				      73  74  75  76  77  78  79  80
+				      81  82  83  84  85  86  87  88
+				      89  90  91  92  93  94  95  96
+				      97  98  99 101 102 103 104 105
+				     106 108 109 110 111 112 114 115
+				     116 117 119 120 121 123 124 125
+				     127 128 129 131 132 133 135 136
+				     138 139 141 142 144 145 147 148
+				     150 151 153 154 156 157 159 161
+				     162 164 166 167 169 171 173 174
+				     176 178 180 181 183 185 187 189
+				     191 192 194 196 198 200 202 204
+				     206 208 210 212 214 216 219 221
+				     223 225 227 229 232 234 236 238
+				     241 242 244 246 248 250 253 255>;
+		default-brightness-level = <128>;
+		enable-gpios = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */
+	};
+
+	panel: panel {
+		compatible = "innolux,at070tn90";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		power-supply = <&panel_power>;
+		backlight = <&backlight>;
+
+		port@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			panel_input: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&tcon0_out_panel>;
+			};
+		};
+	};
+
+	panel_power: panel_power {
+		compatible = "regulator-fixed";
+		regulator-name = "panel-power";
+		regulator-min-microvolt = <10400000>;
+		regulator-max-microvolt = <10400000>;
+		gpio = <&pio 7 8 GPIO_ACTIVE_HIGH>; /* PH8 */
+		enable-active-high;
+		regulator-boot-on;
+	};
+};
+
+&codec {
+	allwinner,pa-gpios = <&pio 7 15 GPIO_ACTIVE_HIGH>; /* PH15 */
+	status = "okay";
+};
+
+&cpu0 {
+	cpu-supply = <&reg_dcdc2>;
+};
+
+&de {
+	status = "okay";
+};
+
+&ehci0 {
+	status = "okay";
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c0_pins_a>;
+	status = "okay";
+
+	axp209: pmic@34 {
+		reg = <0x34>;
+		interrupt-parent = <&nmi_intc>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	};
+};
+
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins_a>;
+	status = "okay";
+
+	lis3dh: accelerometer@18 {
+		compatible = "st,lis3dh-accel";
+		reg = <0x18>;
+		vdd-supply = <&reg_vcc3v3>;
+		vddio-supply = <&reg_vcc3v3>;
+		st,drdy-int-pin = <1>;
+	};
+};
+
+&i2c2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pins_a>;
+	status = "okay";
+	clock-frequency = <400000>;
+
+	gsl1680: touchscreen@40 {
+		compatible = "silead,gsl1680";
+		reg = <0x40>;
+		interrupt-parent = <&pio>;
+		interrupts = <7 21 IRQ_TYPE_EDGE_FALLING>; /* EINT21 (PH21) */
+		power-gpios = <&pio 7 20 GPIO_ACTIVE_HIGH>; /* PH20 */
+		firmware-name = "gsl1680-ainol-aw1.fw";
+		touchscreen-size-x = <480>;
+		touchscreen-size-y = <800>;
+		touchscreen-swapped-x-y;
+		touchscreen-inverted-y;
+		silead,max-fingers = <5>;
+	};
+};
+
+&lradc {
+	vref-supply = <&reg_ldo2>;
+	status = "okay";
+
+	button@571 {
+		label = "Volume Up";
+		linux,code = <KEY_VOLUMEUP>;
+		channel = <0>;
+		voltage = <571428>;
+	};
+
+	button@761 {
+		label = "Volume Down";
+		linux,code = <KEY_VOLUMEDOWN>;
+		channel = <0>;
+		voltage = <761904>;
+	};
+
+	button@952 {
+		label = "Home";
+		linux,code = <KEY_HOME>;
+		channel = <0>;
+		voltage = <952380>;
+	};
+};
+
+&mmc0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_pins_a>;
+	vmmc-supply = <&reg_vcc3v3>;
+	bus-width = <4>;
+	cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
+	cd-inverted;
+	status = "okay";
+};
+
+&ohci0 {
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
+
+&otg_sram {
+	status = "okay";
+};
+
+&pwm {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm0_pins_a>;
+	status = "okay";
+};
+
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd0_rgb666_pins>;
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_panel: endpoint@0 {
+		reg = <0>;
+		remote-endpoint = <&panel_input>;
+	};
+};
+
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_pins_a>;
+	status = "okay";
+};
+
+&usb_otg {
+	dr_mode = "otg";
+	status = "okay";
+};
+
+&usbphy {
+	usb0_id_det-gpio = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
+	usb0_vbus_power-supply = <&usb_power_supply>;
+	usb0_vbus-supply = <&reg_usb0_vbus>;
+	usb1_vbus-supply = <&reg_usb1_vbus>;
+	usb2_vbus-supply = <&reg_usb2_vbus>;
+	status = "okay";
+};
+
+#include "axp209.dtsi"
+
+&battery_power_supply {
+	status = "okay";
+};
+
+&reg_dcdc2 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1450000>;
+	regulator-name = "vdd-cpu";
+};
+
+&reg_dcdc3 {
+	regulator-always-on;
+	regulator-min-microvolt = <1000000>;
+	regulator-max-microvolt = <1400000>;
+	regulator-name = "vdd-int-dll";
+};
+
+&reg_ldo1 {
+	regulator-name = "vdd-rtc";
+};
+
+&reg_ldo2 {
+	regulator-always-on;
+	regulator-min-microvolt = <3000000>;
+	regulator-max-microvolt = <3000000>;
+	regulator-name = "avcc";
+};
+
+&reg_usb0_vbus {
+	status = "okay";
+};
+
+&reg_usb1_vbus {
+	status = "okay";
+};
+
+&reg_usb2_vbus {
+	status = "okay";
+};
+
+&usb_power_supply {
+	status = "okay";
+};
-- 
2.17.0

Re: [PATCH v4 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2536 bytes --]

On Tue, May 08, 2018 at 12:04:13AM +0200, Paul Kocialkowski wrote:
> +++ b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> @@ -0,0 +1,297 @@
> +/*
> + * SPDX-License-Identifier: (GPL-2.0+ OR MIT)

This really should be the first line, and with a C++ style comment, as
in:

// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
 * Copyright (C) ...

See Documentation/process/license-rules.rst

> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> +		brightness-levels = <  0   1   1   1   1   2   2   2
> +				       2   3   3   3   3   4   4   4
> +				       5   5   5   6   6   6   7   7
> +				       8   8   8   9   9   9  10  10
> +				      10  11  11  12  12  12  13  13
> +				      14  14  14  15  15  16  16  17
> +				      17  17  18  18  19  19  20  20
> +				      21  21  21  22  22  23  23  24
> +				      24  25  25  26  26  27  27  28
> +				      28  29  30  30  31  31  32  32
> +				      33  33  34  35  35  36  36  37
> +				      38  38  39  39  40  41  41  42
> +				      43  43  44  44  45  46  47  47
> +				      48  49  49  50  51  51  52  53
> +				      54  54  55  56  57  57  58  59
> +				      60  61  61  62  63  64  65  65
> +				      66  67  68  69  70  71  71  72
> +				      73  74  75  76  77  78  79  80
> +				      81  82  83  84  85  86  87  88
> +				      89  90  91  92  93  94  95  96
> +				      97  98  99 101 102 103 104 105
> +				     106 108 109 110 111 112 114 115
> +				     116 117 119 120 121 123 124 125
> +				     127 128 129 131 132 133 135 136
> +				     138 139 141 142 144 145 147 148
> +				     150 151 153 154 156 157 159 161
> +				     162 164 166 167 169 171 173 174
> +				     176 178 180 181 183 185 187 189
> +				     191 192 194 196 198 200 202 204
> +				     206 208 210 212 214 216 219 221
> +				     223 225 227 229 232 234 236 238
> +				     241 242 244 246 248 250 253 255>;

You kind of overdid it here :)

What I meant to say before was that if you have 10 elements (and you
really should have something in that magnitude) each step should
increase the perceived brightness by 10%.

In this particular case, I really think having something close to <0 4
8 16 32 64 128 255> would be enough.

And in general, that kind of odd looking table without any more
context is just screaming for a comment :)

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 3305 bytes --]

Hi and thanks for the review!

Le vendredi 11 mai 2018 à 16:36 +0200, Maxime Ripard a écrit :
> On Tue, May 08, 2018 at 12:04:13AM +0200, Paul Kocialkowski wrote:
> > +++ b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> > @@ -0,0 +1,297 @@
> > +/*
> > + * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> 
> This really should be the first line, and with a C++ style comment, as
> in:
> 
> // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> /*
>  * Copyright (C) ...
> 
> See Documentation/process/license-rules.rst

Okay, will do in v5.

> > +	backlight: backlight {
> > +		compatible = "pwm-backlight";
> > +		pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> > +		brightness-levels = <  0   1   1   1   1   2   2   2
> > +				       2   3   3   3   3   4   4   4
> > +				       5   5   5   6   6   6   7   7
> > +				       8   8   8   9   9   9  10  10
> > +				      10  11  11  12  12  12  13  13
> > +				      14  14  14  15  15  16  16  17
> > +				      17  17  18  18  19  19  20  20
> > +				      21  21  21  22  22  23  23  24
> > +				      24  25  25  26  26  27  27  28
> > +				      28  29  30  30  31  31  32  32
> > +				      33  33  34  35  35  36  36  37
> > +				      38  38  39  39  40  41  41  42
> > +				      43  43  44  44  45  46  47  47
> > +				      48  49  49  50  51  51  52  53
> > +				      54  54  55  56  57  57  58  59
> > +				      60  61  61  62  63  64  65  65
> > +				      66  67  68  69  70  71  71  72
> > +				      73  74  75  76  77  78  79  80
> > +				      81  82  83  84  85  86  87  88
> > +				      89  90  91  92  93  94  95  96
> > +				      97  98  99 101 102 103 104 105
> > +				     106 108 109 110 111 112 114 115
> > +				     116 117 119 120 121 123 124 125
> > +				     127 128 129 131 132 133 135 136
> > +				     138 139 141 142 144 145 147 148
> > +				     150 151 153 154 156 157 159 161
> > +				     162 164 166 167 169 171 173 174
> > +				     176 178 180 181 183 185 187 189
> > +				     191 192 194 196 198 200 202 204
> > +				     206 208 210 212 214 216 219 221
> > +				     223 225 227 229 232 234 236 238
> > +				     241 242 244 246 248 250 253 255>;
> 
> You kind of overdid it here :)
> 
> What I meant to say before was that if you have 10 elements (and you
> really should have something in that magnitude) each step should
> increase the perceived brightness by 10%.

Mhh I think 10 elements would fall too short to really depict the curve
with appropriate precision. Given the usual size for brightness cursors
in e.g. gnome-shell, it feels like a bigger number would be more
appropriate. Let's make it to 100 with values from 0 to 255!

> In this particular case, I really think having something close to <0 4
> 8 16 32 64 128 255> would be enough.
> 
> And in general, that kind of odd looking table without any more
> context is just screaming for a comment :)

Noted, I will explain the idea, but probably without the exact formula
that's really a nasty hack written down on a piece of paper sitting in
my garbage at this point.

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3005 bytes --]

On Mon, May 14, 2018 at 10:36:08PM +0200, Paul Kocialkowski wrote:
> > > +	backlight: backlight {
> > > +		compatible = "pwm-backlight";
> > > +		pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> > > +		brightness-levels = <  0   1   1   1   1   2   2   2
> > > +				       2   3   3   3   3   4   4   4
> > > +				       5   5   5   6   6   6   7   7
> > > +				       8   8   8   9   9   9  10  10
> > > +				      10  11  11  12  12  12  13  13
> > > +				      14  14  14  15  15  16  16  17
> > > +				      17  17  18  18  19  19  20  20
> > > +				      21  21  21  22  22  23  23  24
> > > +				      24  25  25  26  26  27  27  28
> > > +				      28  29  30  30  31  31  32  32
> > > +				      33  33  34  35  35  36  36  37
> > > +				      38  38  39  39  40  41  41  42
> > > +				      43  43  44  44  45  46  47  47
> > > +				      48  49  49  50  51  51  52  53
> > > +				      54  54  55  56  57  57  58  59
> > > +				      60  61  61  62  63  64  65  65
> > > +				      66  67  68  69  70  71  71  72
> > > +				      73  74  75  76  77  78  79  80
> > > +				      81  82  83  84  85  86  87  88
> > > +				      89  90  91  92  93  94  95  96
> > > +				      97  98  99 101 102 103 104 105
> > > +				     106 108 109 110 111 112 114 115
> > > +				     116 117 119 120 121 123 124 125
> > > +				     127 128 129 131 132 133 135 136
> > > +				     138 139 141 142 144 145 147 148
> > > +				     150 151 153 154 156 157 159 161
> > > +				     162 164 166 167 169 171 173 174
> > > +				     176 178 180 181 183 185 187 189
> > > +				     191 192 194 196 198 200 202 204
> > > +				     206 208 210 212 214 216 219 221
> > > +				     223 225 227 229 232 234 236 238
> > > +				     241 242 244 246 248 250 253 255>;
> > 
> > You kind of overdid it here :)
> > 
> > What I meant to say before was that if you have 10 elements (and you
> > really should have something in that magnitude) each step should
> > increase the perceived brightness by 10%.
> 
> Mhh I think 10 elements would fall too short to really depict the curve
> with appropriate precision. Given the usual size for brightness cursors
> in e.g. gnome-shell, it feels like a bigger number would be more
> appropriate. Let's make it to 100 with values from 0 to 255!
> 
> > In this particular case, I really think having something close to <0 4
> > 8 16 32 64 128 255> would be enough.
> > 
> > And in general, that kind of odd looking table without any more
> > context is just screaming for a comment :)
> 
> Noted, I will explain the idea, but probably without the exact formula
> that's really a nasty hack written down on a piece of paper sitting in
> my garbage at this point.

So no one will ever be able to understand where this sequence comes
from (yourself-in-two-years included). That sounds like a pretty bad
idea.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [linux-sunxi] Re: [PATCH v4 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet

[Brüns, Stefan]

On Freitag, 18. Mai 2018 09:14:36 CEST Maxime Ripard wrote:
> On Mon, May 14, 2018 at 10:36:08PM +0200, Paul Kocialkowski wrote:
> > > > +	backlight: backlight {
> > > > +		compatible = "pwm-backlight";
> > > > +		pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> > > > +		brightness-levels = <  0   1   1   1   1   2   2   2
> > > > +				       2   3   3   3   3   4   4   4
> > > > +				       5   5   5   6   6   6   7   7
> > > > +				       8   8   8   9   9   9  10  10
> > > > +				      10  11  11  12  12  12  13  13
> > > > +				      14  14  14  15  15  16  16  17
> > > > +				      17  17  18  18  19  19  20  20
> > > > +				      21  21  21  22  22  23  23  24
> > > > +				      24  25  25  26  26  27  27  28
> > > > +				      28  29  30  30  31  31  32  32
> > > > +				      33  33  34  35  35  36  36  37
> > > > +				      38  38  39  39  40  41  41  42
> > > > +				      43  43  44  44  45  46  47  47
> > > > +				      48  49  49  50  51  51  52  53
> > > > +				      54  54  55  56  57  57  58  59
> > > > +				      60  61  61  62  63  64  65  65
> > > > +				      66  67  68  69  70  71  71  72
> > > > +				      73  74  75  76  77  78  79  80
> > > > +				      81  82  83  84  85  86  87  88
> > > > +				      89  90  91  92  93  94  95  96
> > > > +				      97  98  99 101 102 103 104 105
> > > > +				     106 108 109 110 111 112 114 115
> > > > +				     116 117 119 120 121 123 124 125
> > > > +				     127 128 129 131 132 133 135 136
> > > > +				     138 139 141 142 144 145 147 148
> > > > +				     150 151 153 154 156 157 159 161
> > > > +				     162 164 166 167 169 171 173 174
> > > > +				     176 178 180 181 183 185 187 189
> > > > +				     191 192 194 196 198 200 202 204
> > > > +				     206 208 210 212 214 216 219 221
> > > > +				     223 225 227 229 232 234 236 238
> > > > +				     241 242 244 246 248 250 253 255>;
> > > 
> > > You kind of overdid it here :)
> > > 
> > > What I meant to say before was that if you have 10 elements (and you
> > > really should have something in that magnitude) each step should
> > > increase the perceived brightness by 10%.
> > 
> > Mhh I think 10 elements would fall too short to really depict the curve
> > with appropriate precision. Given the usual size for brightness cursors
> > in e.g. gnome-shell, it feels like a bigger number would be more
> > appropriate. Let's make it to 100 with values from 0 to 255!
> > 
> > > In this particular case, I really think having something close to <0 4
> > > 8 16 32 64 128 255> would be enough.
> > > 
> > > And in general, that kind of odd looking table without any more
> > > context is just screaming for a comment :)
> > 
> > Noted, I will explain the idea, but probably without the exact formula
> > that's really a nasty hack written down on a piece of paper sitting in
> > my garbage at this point.
> 
> So no one will ever be able to understand where this sequence comes
> from (yourself-in-two-years included). That sounds like a pretty bad
> idea.
> 
> Maxime

The following formula yields practically the same table:

out = ceil(255 * (0.245 * in/255  +  0.755 * pow(in/255, 2.6) ))

Maximum error: 4, maximum relative error: 0.33 

Kind regards,

Stefan

Re: [PATCH v4 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]

On Tue, May 08, 2018 at 12:04:11AM +0200, Paul Kocialkowski wrote:
> This adds timings for the RGB666 variant of the Innolux AT070TN90 panel,
> as found on the Ainol AW1 tablet.
> 
> The panel also supports RGB888 output. When RGB666 mode is used instead,
> the two extra lanes per component are grounded.
> 
> In the future, it might become necessary to introduce a dedicated
> device-tree property to specify the bus format to use instead of the
> default one for the panel. This will allow supporting different bus
> formats for the same panel modes.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index cbf1ab404ee7..32e30d5a8f08 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1063,6 +1063,29 @@ static const struct panel_desc innolux_at043tn24 = {
>  	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
>  };
>  
> +static const struct drm_display_mode innolux_at070tn90_mode = {
> +	.clock = 40000,
> +	.hdisplay = 800,
> +	.hsync_start = 800 + 112,
> +	.hsync_end = 800 + 112 + 1,
> +	.htotal = 800 + 112 + 1 + 87,
> +	.vdisplay = 480,
> +	.vsync_start = 480 + 141,
> +	.vsync_end = 480 + 141 + 1,
> +	.vtotal = 480 + 141 + 1 + 38,
> +	.vrefresh = 60,
> +};
> +
> +static const struct panel_desc innolux_at070tn90 = {
> +	.modes = &innolux_at070tn90_mode,
> +	.num_modes = 1,
> +	.size = {
> +		.width = 154,
> +		.height = 86,
> +	},
> +	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> +};
> +

I'm not really convinced this is the right approach. You said it
yourself, the panel is using a 24-bits interface, and you just happen
to have a tablet that routed it using a 18-bits interface instead.

That doesn't belong in the driver, especially associated to the
compatible, but where the routing is described: in the device
tree. And given that the panel interface is a 24 bits panel, if we
were to have a default, we should have this one, and not the one
fitting your use case.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90

[Paul Kocialkowski]

Hi,

On Wed, 2018-05-09 at 09:12 +0200, Maxime Ripard wrote:
> On Tue, May 08, 2018 at 12:04:11AM +0200, Paul Kocialkowski wrote:
> > This adds timings for the RGB666 variant of the Innolux AT070TN90 panel,
> > as found on the Ainol AW1 tablet.
> > 
> > The panel also supports RGB888 output. When RGB666 mode is used instead,
> > the two extra lanes per component are grounded.
> > 
> > In the future, it might become necessary to introduce a dedicated
> > device-tree property to specify the bus format to use instead of the
> > default one for the panel. This will allow supporting different bus
> > formats for the same panel modes.
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > index cbf1ab404ee7..32e30d5a8f08 100644
> > --- a/drivers/gpu/drm/panel/panel-simple.c
> > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > @@ -1063,6 +1063,29 @@ static const struct panel_desc innolux_at043tn24 = {
> >  	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> >  };
> >  
> > +static const struct drm_display_mode innolux_at070tn90_mode = {
> > +	.clock = 40000,
> > +	.hdisplay = 800,
> > +	.hsync_start = 800 + 112,
> > +	.hsync_end = 800 + 112 + 1,
> > +	.htotal = 800 + 112 + 1 + 87,
> > +	.vdisplay = 480,
> > +	.vsync_start = 480 + 141,
> > +	.vsync_end = 480 + 141 + 1,
> > +	.vtotal = 480 + 141 + 1 + 38,
> > +	.vrefresh = 60,
> > +};
> > +
> > +static const struct panel_desc innolux_at070tn90 = {
> > +	.modes = &innolux_at070tn90_mode,
> > +	.num_modes = 1,
> > +	.size = {
> > +		.width = 154,
> > +		.height = 86,
> > +	},
> > +	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > +};
> > +
> 
> I'm not really convinced this is the right approach. You said it
> yourself, the panel is using a 24-bits interface, and you just happen
> to have a tablet that routed it using a 18-bits interface instead.
> 
> That doesn't belong in the driver, especially associated to the
> compatible, but where the routing is described: in the device
> tree. And given that the panel interface is a 24 bits panel, if we
> were to have a default, we should have this one, and not the one
> fitting your use case.

I fully agree, this is why I suggested introducing a dedicated dt
property for selecting the bus format (in the commit message). I still
proposed this patch as a temporary solution, but I'm definitely willing
to craft a proper solution as well.

Here is an initial proposition:
1. Making bus_format an array in struct panel_desc and listing all the
relevant bus formats that the panel can support there;
2. Introducing an optional "bus-format" dt property that indicates which
bus format to use, and using the first index of the bus formats array if
the property is not present;
3. Checking that the bus format requested from dt is supported by the
panel.

What do you think?

Paul

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

Re: [PATCH v4 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3758 bytes --]

On Wed, May 09, 2018 at 01:31:23PM +0200, Paul Kocialkowski wrote:
> On Wed, 2018-05-09 at 09:12 +0200, Maxime Ripard wrote:
> > On Tue, May 08, 2018 at 12:04:11AM +0200, Paul Kocialkowski wrote:
> > > This adds timings for the RGB666 variant of the Innolux AT070TN90 panel,
> > > as found on the Ainol AW1 tablet.
> > > 
> > > The panel also supports RGB888 output. When RGB666 mode is used instead,
> > > the two extra lanes per component are grounded.
> > > 
> > > In the future, it might become necessary to introduce a dedicated
> > > device-tree property to specify the bus format to use instead of the
> > > default one for the panel. This will allow supporting different bus
> > > formats for the same panel modes.
> > > 
> > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > ---
> > >  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> > >  1 file changed, 26 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > index cbf1ab404ee7..32e30d5a8f08 100644
> > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > @@ -1063,6 +1063,29 @@ static const struct panel_desc innolux_at043tn24 = {
> > >  	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > >  };
> > >  
> > > +static const struct drm_display_mode innolux_at070tn90_mode = {
> > > +	.clock = 40000,
> > > +	.hdisplay = 800,
> > > +	.hsync_start = 800 + 112,
> > > +	.hsync_end = 800 + 112 + 1,
> > > +	.htotal = 800 + 112 + 1 + 87,
> > > +	.vdisplay = 480,
> > > +	.vsync_start = 480 + 141,
> > > +	.vsync_end = 480 + 141 + 1,
> > > +	.vtotal = 480 + 141 + 1 + 38,
> > > +	.vrefresh = 60,
> > > +};
> > > +
> > > +static const struct panel_desc innolux_at070tn90 = {
> > > +	.modes = &innolux_at070tn90_mode,
> > > +	.num_modes = 1,
> > > +	.size = {
> > > +		.width = 154,
> > > +		.height = 86,
> > > +	},
> > > +	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > > +};
> > > +
> > 
> > I'm not really convinced this is the right approach. You said it
> > yourself, the panel is using a 24-bits interface, and you just happen
> > to have a tablet that routed it using a 18-bits interface instead.
> > 
> > That doesn't belong in the driver, especially associated to the
> > compatible, but where the routing is described: in the device
> > tree. And given that the panel interface is a 24 bits panel, if we
> > were to have a default, we should have this one, and not the one
> > fitting your use case.
> 
> I fully agree, this is why I suggested introducing a dedicated dt
> property for selecting the bus format (in the commit message). I still
> proposed this patch as a temporary solution, but I'm definitely willing
> to craft a proper solution as well.
> 
> Here is an initial proposition:
> 1. Making bus_format an array in struct panel_desc and listing all the
> relevant bus formats that the panel can support there;

I'm not sure this is needed, the input format is always the same in
your case, the panel will always take a 24 bits RGB value. What you
want to change is the encoder output format (and I guess you want that
to be meaningful to enable or not the dithering).

> 2. Introducing an optional "bus-format" dt property that indicates which
> bus format to use, and using the first index of the bus formats array if
> the property is not present;

I guess the width would be enough, and that way we can take the
bus-width format that is already defined (but used in the v4l2
framework, not in DRM yet).

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 4626 bytes --]

Hi,

Le vendredi 11 mai 2018 à 10:59 +0200, Maxime Ripard a écrit :
> On Wed, May 09, 2018 at 01:31:23PM +0200, Paul Kocialkowski wrote:
> > On Wed, 2018-05-09 at 09:12 +0200, Maxime Ripard wrote:
> > > On Tue, May 08, 2018 at 12:04:11AM +0200, Paul Kocialkowski wrote:
> > > > This adds timings for the RGB666 variant of the Innolux AT070TN90 panel,
> > > > as found on the Ainol AW1 tablet.
> > > > 
> > > > The panel also supports RGB888 output. When RGB666 mode is used instead,
> > > > the two extra lanes per component are grounded.
> > > > 
> > > > In the future, it might become necessary to introduce a dedicated
> > > > device-tree property to specify the bus format to use instead of the
> > > > default one for the panel. This will allow supporting different bus
> > > > formats for the same panel modes.
> > > > 
> > > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > > ---
> > > >  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> > > >  1 file changed, 26 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > > index cbf1ab404ee7..32e30d5a8f08 100644
> > > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > > @@ -1063,6 +1063,29 @@ static const struct panel_desc innolux_at043tn24 = {
> > > >  	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > > >  };
> > > >  
> > > > +static const struct drm_display_mode innolux_at070tn90_mode = {
> > > > +	.clock = 40000,
> > > > +	.hdisplay = 800,
> > > > +	.hsync_start = 800 + 112,
> > > > +	.hsync_end = 800 + 112 + 1,
> > > > +	.htotal = 800 + 112 + 1 + 87,
> > > > +	.vdisplay = 480,
> > > > +	.vsync_start = 480 + 141,
> > > > +	.vsync_end = 480 + 141 + 1,
> > > > +	.vtotal = 480 + 141 + 1 + 38,
> > > > +	.vrefresh = 60,
> > > > +};
> > > > +
> > > > +static const struct panel_desc innolux_at070tn90 = {
> > > > +	.modes = &innolux_at070tn90_mode,
> > > > +	.num_modes = 1,
> > > > +	.size = {
> > > > +		.width = 154,
> > > > +		.height = 86,
> > > > +	},
> > > > +	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > > > +};
> > > > +
> > > 
> > > I'm not really convinced this is the right approach. You said it
> > > yourself, the panel is using a 24-bits interface, and you just happen
> > > to have a tablet that routed it using a 18-bits interface instead.
> > > 
> > > That doesn't belong in the driver, especially associated to the
> > > compatible, but where the routing is described: in the device
> > > tree. And given that the panel interface is a 24 bits panel, if we
> > > were to have a default, we should have this one, and not the one
> > > fitting your use case.
> > 
> > I fully agree, this is why I suggested introducing a dedicated dt
> > property for selecting the bus format (in the commit message). I still
> > proposed this patch as a temporary solution, but I'm definitely willing
> > to craft a proper solution as well.
> > 
> > Here is an initial proposition:
> > 1. Making bus_format an array in struct panel_desc and listing all the
> > relevant bus formats that the panel can support there;
> 
> I'm not sure this is needed, the input format is always the same in
> your case, the panel will always take a 24 bits RGB value. What you
> want to change is the encoder output format (and I guess you want that
> to be meaningful to enable or not the dithering).

Isn't the panel format supposed to match what the encoder's output
should be aiming for? In my case, that would be RGB666, so the idea
would be specifying both MEDIA_BUS_FMT_RGB666_1X18 and
MEDIA_BUS_FMT_RGB888_1X24 in a list of supported bus formats for the
panel. This way, both my setup and RGB888 setups can be supported.

> > 2. Introducing an optional "bus-format" dt property that indicates which
> > bus format to use, and using the first index of the bus formats array if
> > the property is not present;
> 
> I guess the width would be enough, and that way we can take the
> bus-width format that is already defined (but used in the v4l2
> framework, not in DRM yet).

Well, we already have bus-format defines on the DRM side and it feels
like mapping these directly in device-tree would be more useful as a
description of the hardware than just having the bus width.

Cheers,

Paul

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5351 bytes --]

On Mon, May 14, 2018 at 10:40:15PM +0200, Paul Kocialkowski wrote:
> Le vendredi 11 mai 2018 à 10:59 +0200, Maxime Ripard a écrit :
> > On Wed, May 09, 2018 at 01:31:23PM +0200, Paul Kocialkowski wrote:
> > > On Wed, 2018-05-09 at 09:12 +0200, Maxime Ripard wrote:
> > > > On Tue, May 08, 2018 at 12:04:11AM +0200, Paul Kocialkowski wrote:
> > > > > This adds timings for the RGB666 variant of the Innolux AT070TN90 panel,
> > > > > as found on the Ainol AW1 tablet.
> > > > > 
> > > > > The panel also supports RGB888 output. When RGB666 mode is used instead,
> > > > > the two extra lanes per component are grounded.
> > > > > 
> > > > > In the future, it might become necessary to introduce a dedicated
> > > > > device-tree property to specify the bus format to use instead of the
> > > > > default one for the panel. This will allow supporting different bus
> > > > > formats for the same panel modes.
> > > > > 
> > > > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > > > ---
> > > > >  drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> > > > >  1 file changed, 26 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > > > index cbf1ab404ee7..32e30d5a8f08 100644
> > > > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > > > @@ -1063,6 +1063,29 @@ static const struct panel_desc innolux_at043tn24 = {
> > > > >  	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> > > > >  };
> > > > >  
> > > > > +static const struct drm_display_mode innolux_at070tn90_mode = {
> > > > > +	.clock = 40000,
> > > > > +	.hdisplay = 800,
> > > > > +	.hsync_start = 800 + 112,
> > > > > +	.hsync_end = 800 + 112 + 1,
> > > > > +	.htotal = 800 + 112 + 1 + 87,
> > > > > +	.vdisplay = 480,
> > > > > +	.vsync_start = 480 + 141,
> > > > > +	.vsync_end = 480 + 141 + 1,
> > > > > +	.vtotal = 480 + 141 + 1 + 38,
> > > > > +	.vrefresh = 60,
> > > > > +};
> > > > > +
> > > > > +static const struct panel_desc innolux_at070tn90 = {
> > > > > +	.modes = &innolux_at070tn90_mode,
> > > > > +	.num_modes = 1,
> > > > > +	.size = {
> > > > > +		.width = 154,
> > > > > +		.height = 86,
> > > > > +	},
> > > > > +	.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > > > > +};
> > > > > +
> > > > 
> > > > I'm not really convinced this is the right approach. You said it
> > > > yourself, the panel is using a 24-bits interface, and you just happen
> > > > to have a tablet that routed it using a 18-bits interface instead.
> > > > 
> > > > That doesn't belong in the driver, especially associated to the
> > > > compatible, but where the routing is described: in the device
> > > > tree. And given that the panel interface is a 24 bits panel, if we
> > > > were to have a default, we should have this one, and not the one
> > > > fitting your use case.
> > > 
> > > I fully agree, this is why I suggested introducing a dedicated dt
> > > property for selecting the bus format (in the commit message). I still
> > > proposed this patch as a temporary solution, but I'm definitely willing
> > > to craft a proper solution as well.
> > > 
> > > Here is an initial proposition:
> > > 1. Making bus_format an array in struct panel_desc and listing all the
> > > relevant bus formats that the panel can support there;
> > 
> > I'm not sure this is needed, the input format is always the same in
> > your case, the panel will always take a 24 bits RGB value. What you
> > want to change is the encoder output format (and I guess you want that
> > to be meaningful to enable or not the dithering).
> 
> Isn't the panel format supposed to match what the encoder's output
> should be aiming for? In my case, that would be RGB666, so the idea
> would be specifying both MEDIA_BUS_FMT_RGB666_1X18 and
> MEDIA_BUS_FMT_RGB888_1X24 in a list of supported bus formats for the
> panel.

The width your panel has in input is in 24 bits. The width the encoder
outputs in is 16 bits. This is the panel driver, you should expose the
panel capabilities.

> This way, both my setup and RGB888 setups can be supported.

I don't see what prevents you to do that with my suggestion either.

> > > 2. Introducing an optional "bus-format" dt property that indicates which
> > > bus format to use, and using the first index of the bus formats array if
> > > the property is not present;
> > 
> > I guess the width would be enough, and that way we can take the
> > bus-width format that is already defined (but used in the v4l2
> > framework, not in DRM yet).
> 
> Well, we already have bus-format defines on the DRM side and it feels
> like mapping these directly in device-tree would be more useful as a
> description of the hardware than just having the bus width.

Having the format in the DT doesn't make much sense. A given panel can
support multiple formats, just like a given encoder can.

If you're in that situation, the DT would describe a policy over what
happens in the OS, which isn't what should be stored in the DT. The
bus width, on the other end, is a property of the hardware.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]