Hi Andrew, On 25-Apr-23 23:14, Andrew Davis wrote: > On 4/25/23 12:00 PM, Aradhya Bhatia wrote: >> Hi Andrew, >> >> On 25-Apr-23 20:39, Andrew Davis wrote: >>> On 4/25/23 12:12 AM, Aradhya Bhatia wrote: >>>> From: Jyri Sarha <jsarha@xxxxxx> >>>> >>>> The OLDI-LCD1EVM add on board has Rocktech RK101II01D-CT panel with >>>> integrated touch screen. The integrated touch screen is Goodix GT928. >>>> Add DT nodes for these and connect the endpoint nodes with DSS. >>>> >>>> This patch was picked from TI's public tree based on 5.10 LTS kernel. >>>> >>>> Signed-off-by: Jyri Sarha <jsarha@xxxxxx> >>>> Signed-off-by: Nikhil Devshatwar <nikhil.nd@xxxxxx> >>>> [abhatia1@xxxxxx: Make syntax changes to support 6.1 DTSO format] >>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@xxxxxx> >>>> --- >>>> arch/arm64/boot/dts/ti/Makefile | 2 + >>>> .../dts/ti/k3-am654-evm-oldi-lcd1evm.dtso | 70 >>>> +++++++++++++++++++ >>>> 2 files changed, 72 insertions(+) >>>> create mode 100644 >>>> arch/arm64/boot/dts/ti/k3-am654-evm-oldi-lcd1evm.dtso >>>> >>>> diff --git a/arch/arm64/boot/dts/ti/Makefile >>>> b/arch/arm64/boot/dts/ti/Makefile >>>> index 6acd12409d59..8956b19e587a 100644 >>>> --- a/arch/arm64/boot/dts/ti/Makefile >>>> +++ b/arch/arm64/boot/dts/ti/Makefile >>>> @@ -26,6 +26,7 @@ dtb-$(CONFIG_ARCH_K3) += >>>> k3-am6548-iot2050-advanced.dtb >>>> dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced-m2.dtb >>>> dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced-pg2.dtb >>>> dtb-$(CONFIG_ARCH_K3) += k3-am654-base-board.dtb >>>> +dtb-$(CONFIG_ARCH_K3) += k3-am654-evm-oldi-lcd1evm.dtbo >>> >>> This name is a bit odd, why "evm" twice? Looks like the first instance >>> is the redundant one as most of the documents on this LCD board call it >>> the "LCD1EVM". How about: >>> >>> k3-am654-lcd1evm.dtbo >> >> I didn't think I could change the name of the overlay picking the patch >> from our tree, but if we are going to do it, can we take up another >> approach, where it would be easier to add panels for AM62x family and >> ensure uniformity throughout. >> > > Yes, we can change what we want when upstreaming. How we did it in our > evil vendor tree should in no way prevent us from doing things better > in upstream. > Okay. >> We have 2 different panels from Vendor A, and another one from Vendor B. >> Vendor B panel connects to AM625-SK via an adapter board. >> >> Vendor-A/Panel-1 only says the name, 'SK-LCD1' on its circuit board. >> Vendor-A/Panel-2 doesn't have any name yet. We only have development >> units. >> Vendor-B/Panel-2 mentions '$(LCD_model) to AM62x SoC adapter board'. >> >> Since, there are too many manufacturers, it is difficult to maintain >> uniformity with the names of panel-boards. So, I have this approach in >> mind (which I have used for our tree for AM62x), but would like your >> comments. >> >> k3-$soc-$board-$(panel_vendor)-$(brief_compatible).dtso >> >> So, for AM625-SKs, >> k3-am625-sk-$(vendor_name)-$(brief_compatible).dtso >> > > Looks reasonable to me. Great! I tweaked this to add the keyword "panel" to also make it reader friendly in v2. k3-$soc-$board-$(panel_vendor)-$(brief_compatible)-panel.dtso > >> and for the current panel Rocktech RK101II01D-CT, which applies on AM654 >> base-board, >> >> k3-am654-base-board-rocktech-rk101.dtso. >> >> This does become rather long, but also is distinguishable. >> > > No limit to file names here, being clear and distinguishable is more > important than short names. > >> Let me know what you think. >> >>> >>> I would like the overlay names to give some hint to what base DTB they >>> apply to, >> >> Agreed. That is indeed how it should be. >> >>> or better yet, apply them here in the build which will check >>> that they apply cleanly. Plus you can drop the silly "+= -@" below. >>> >> >> The above approach will give a hint of the base EVM where a combined >> build is not possible simply because there is no 'official' name for a >> particular combination of panel and EVM. >> > > We do not need to name each possible combination (and we shouldn't, there > would be a combinatorial explosion, avoiding that is the whole point > of using overlays vs .dtsi includes). > > I do think we should name at least the combinations that we ship together. > So as below for AM654 that would be the GPEVM and the IDK. Those are the > two out-of-box combinations available for purchase as a kit. Folks can > still > buy additional add-on cards, and/or mix and match from those two sets. > > As long as we have at least one named combination, then the base-board dtb > file gets symbols automatically and we can drop the "+= -@" line. > Understood! I have incorporated these suggestions, as well as the ones from Tomi and Nishanth, in v2 and posted them. Regards Aradhya > Andrew > >> >> Regards >> Aradhya >> >>> Let's see how this should be called, from the AM65x GP EVM doc[0] we >>> get a nice picture on page 5 and the following: >>> >>> "The AM65x GP EVM consists of a common processor board, an LCD adapter, >>> and a one-lane PCIe/USB3 personality card." >>> >>> So, this would translate to: >>> >>> k3-am654-gp-evm-dtbs := k3-am654-base-board.dtb k3-am654-lcd1evm.dtbo >>> k3-am654-pcie-usb3.dtbo >>> dtb-$(CONFIG_ARCH_K3) += k3-am654-gp-evm.dtb> >>> Next, from the AM65x IDK doc[1] also with a nice image on page 5: >>> >>> "The AM65x IDK consists of a common processor board, IDK application >>> board, >>> and a two-lane PCIe personality card.: >>> >>> So: >>> >>> k3-am654-idk-dtbs := k3-am654-base-board.dtb k3-am654-idk.dtbo >>> k3-am654-pcie-usb2.dtbo >>> dtb-$(CONFIG_ARCH_K3) += k3-am654-idk.dtb >>> >>> Note that we do have all those missing dtso files in our evil vendor >>> tree[2] >>> and will be upstreaming them next, so this naming should all work out >>> nicely. >>> >>> Andrew >>> >>> [0] https://www.ti.com/lit/ug/spruim7/spruim7.pdf >>> [1] https://www.ti.com/lit/ug/spruim6a/spruim6a.pdf >>> [2] >>> https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/arch/arm64/boot/dts/ti?h=ti-linux-5.10.y >>> >>>> # Boards with J7200 SoC >>>> dtb-$(CONFIG_ARCH_K3) += k3-j7200-common-proc-board.dtb >>>> @@ -45,3 +46,4 @@ dtb-$(CONFIG_ARCH_K3) += k3-j784s4-evm.dtb >>>> # Enable support for device-tree overlays >>>> DTC_FLAGS_k3-am6548-iot2050-advanced-m2 += -@ >>>> +DTC_FLAGS_k3-am654-base-board += -@ >>>> diff --git a/arch/arm64/boot/dts/ti/k3-am654-evm-oldi-lcd1evm.dtso >>>> b/arch/arm64/boot/dts/ti/k3-am654-evm-oldi-lcd1evm.dtso >>>> new file mode 100644 >>>> index 000000000000..b2c790b314cf >>>> --- /dev/null >>>> +++ b/arch/arm64/boot/dts/ti/k3-am654-evm-oldi-lcd1evm.dtso >>>> @@ -0,0 +1,70 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/** >>>> + * OLDI-LCD1EVM Rocktech integrated panel and touch DT overlay for >>>> AM654-EVM. >>>> + * >>>> + * Copyright (C) 2023 Texas Instruments Incorporated - >>>> http://www.ti.com/ >>>> + */ >>>> + >>>> +/dts-v1/; >>>> +/plugin/; >>>> + >>>> +#include <dt-bindings/pwm/pwm.h> >>>> +#include <dt-bindings/gpio/gpio.h> >>>> +#include <dt-bindings/interrupt-controller/irq.h> >>>> + >>>> +&{/} { >>>> + display0 { >>>> + compatible = "rocktech,rk101ii01d-ct"; >>>> + backlight = <&lcd_bl>; >>>> + enable-gpios = <&pca9555 8 GPIO_ACTIVE_HIGH>; >>>> + port { >>>> + lcd_in0: endpoint { >>>> + remote-endpoint = <&oldi_out0>; >>>> + }; >>>> + }; >>>> + }; >>>> + >>>> + lcd_bl: backlight { >>>> + compatible = "pwm-backlight"; >>>> + pwms = <&ecap0 0 50000 PWM_POLARITY_INVERTED>; >>>> + brightness-levels = >>>> + <0 32 64 96 128 160 192 224 255>; >>>> + default-brightness-level = <8>; >>>> + }; >>>> +}; >>>> + >>>> +&dss { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&dss_ports { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + port@0 { >>>> + reg = <0>; >>>> + >>>> + oldi_out0: endpoint { >>>> + remote-endpoint = <&lcd_in0>; >>>> + }; >>>> + }; >>>> +}; >>>> + >>>> +&main_i2c1 { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + gt928: touchscreen@14 { >>>> + status = "okay"; >>>> + compatible = "goodix,gt928"; >>>> + reg = <0x14>; >>>> + >>>> + interrupt-parent = <&pca9554>; >>>> + interrupts = <3 IRQ_TYPE_EDGE_FALLING>; >>>> + touchscreen-size-x = <1280>; >>>> + touchscreen-size-y = <800>; >>>> + >>>> + reset-gpios = <&pca9555 9 GPIO_ACTIVE_HIGH>; >>>> + irq-gpios = <&pca9554 3 GPIO_ACTIVE_HIGH>; >>>> + }; >>>> +};