rockchip64: fix wrong GPIO direction in DP virtual extcon driver

[retro98boy]

Description

When I tried to light up the HDMI screen on a RK3399 Box, I found that it was actually driven by Type-C DP Alt mode. So I used the miniDP-virtual-extcon patch in the build repository.

Modify the device tree:

	virtual_pd: virtual-pd {
		compatible = "linux,extcon-usbc-virtual-pd";
		det-gpios = <&gpio4 RK_PD1 GPIO_ACTIVE_LOW>;
		vpd-data-role = "display-port";
		vpd-super-speed;
	};

After compiling the miniDP-virtual-extcon driver and insmoding it, it reported an error:

[  732.569785] gpio gpiochip4: (gpio4): gpiochip_lock_as_irq: tried to flag a GPIO set as output for IRQ
[  732.570687] gpio gpiochip4: (gpio4): unable to lock HW IRQ 25 for IRQ
[  732.571298] genirq: Failed to request resources for virtual-pd (irq 88) on irqchip rockchip_gpio_irq
[  732.572366] extcon-usbc-virtual-pd virtual-pd: failed to request gpio irq
[  732.573220] extcon-usbc-virtual-pd virtual-pd: probe with driver extcon-usbc-virtual-pd failed with error -5

Then I modified the det GPIO direction in the driver to input, recompiled and loaded it. The driver can be initialized and the HDMI screen also works normally. This is dmesg:

[  582.207553] rockchip-drm display-subsystem: bound ff8f0000.vop (ops 0xffffad505abc0b50)
[  582.209131] rockchip-drm display-subsystem: bound ff900000.vop (ops 0xffffad505abc0b50)
[  582.209949] rockchip-drm display-subsystem: bound fec00000.dp (ops 0xffffad505abc5158)
[  582.212791] [drm] Initialized rockchip 1.0.0 for display-subsystem on minor 1
[  582.212947] rockchip-drm display-subsystem: [drm] Cannot find any crtc or sizes
[  582.215185] rockchip-drm display-subsystem: [drm] Cannot find any crtc or sizes
[  582.215766] cdn-dp fec00000.dp: [drm:cdn_dp_pd_event_work [rockchipdrm]] Connected, not enabled; enabling cdn

How Has This Been Tested?

• After applying the patch in the kernel source code, the driver can be enabled and compiled normally. After the driver is loaded in RK3399 Box, HDMI works normally.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• Any dependent changes have been merged and published in downstream modules

[paolosabatino]

On my rk3399 board (OrangePi 4 LTS) the HDMI is also wired through DP Alt, but the driver is not needed AFAIK. As far as I can remember, this was the patch to make it work.

Actually I don't know if this driver or the other patch are "the right way to go" for mainline kernel, but it seems that the other way wires upon already existing things in the kernel and device trees.

I have nothing against this solution, but your mileage may vary in the future if this is "non-standard".

[retro98boy]

On my rk3399 board (OrangePi 4 LTS) the HDMI is also wired through DP Alt, but the driver is not needed AFAIK. As far as I can remember, this was the patch to make it work.

Actually I don't know if this driver or the other patch are "the right way to go" for mainline kernel, but it seems that the other way wires upon already existing things in the kernel and device trees.

I have nothing against this solution, but your mileage may vary in the future if this is "non-standard".

My board was bought from the second-hand market, and there is no specific schematic diagram. I extracted the dtb of the BSP kernel from the original Android system, then decompiled it into dts, and rewritten the dts of the mainline kernel version.

In the dts I wrote, enabling the HDMI node does not make the board's HDMI work.

In the decompiled dts, there is such a node:

    virtual-pd0 {
        compatible = "linux,extcon-pd-virtual";
        dp-det-gpios = <0x000000d7 0x00000019 0x00000001>;
        hdmi-5v-gpios = <0x000000d7 0x0000001c 0x00000001>;
        vpd,init-flip = <0x00000000>;
        vpd,init-ss = <0x00000000>;
        vpd,init-mode = <0x00000003>;
        linux,phandle = <0x0000002f>;
        phandle = <0x0000002f>;
    };

I googled linux,extcon-pd-virtual hoping to get some information and found this patch

At this time, I realized that the HDMI physical interface on the board is not native, but converted from USB Type-C. This USB Type-C is designed to fixedly output DP signals, and it does not have a Type-C control chip similar to FUSB302. It only uses a GPIO to detect whether the HDMI cable is inserted.

The function of the above patch is to tell the cdn-dp driver of RK3399 to output the DP signal at the appropriate time (GPIO is pulled low). It seems like this patch was not accepted by the mainline kernel, but I found it in Armbian's repository and tried using it, and it worked fine on my board.

I found that a similar driver exists for rockchip's BSP kernel. It is the driver originally used by the Android system of my board. It should come from the rockchip manufacturer.

[paolosabatino]

Ok now I understand it better and looking again at the diff, I see that your change is swapping the gpio default from output + down to input. input seems to me too the right choice if you really want to do an autodection, since in the driver code at some point it reads the status of the pin.

I have two doubts though:

• perhaps, a better way to handle this is via pinctrl properties in the device tree, to specify gpio direction and pull-up/pull-down instead of hardcoding it in the driver
• most important: if the pin was output and now becomes input, how it is possible it was working on existing boards that currently use this driver (if any)? I mean, could this change break HDMI on some existing/supported boards?

[retro98boy]

Ok now I understand it better and looking again at the diff, I see that your change is swapping the gpio default from output + down to input. input seems to me too the right choice if you really want to do an autodection, since in the driver code at some point it reads the status of the pin.

I have two doubts though:

* perhaps, a better way to handle this is via `pinctrl` properties in the device tree, to specify gpio direction and pull-up/pull-down instead of hardcoding it in the driver

* **most important:** if the pin was output and now becomes input, how it is possible it was working on existing boards that currently use this driver (if any)? I mean, could this change break HDMI on some existing/supported boards?

I was ashamed that I forgot to set the GPIO pinctrl to input in dts, so when I got home I immediately set it in dts:

&pinctrl {
......
	display {
		dp-hpd {
			rockchip,pins = <4 RK_PD1 RK_FUNC_GPIO &pcfg_input_pull_up>;
		};

		hdmi_5v_en: hdmi-5v-en {
			rockchip,pins = <4 RK_PD4 RK_FUNC_GPIO &pcfg_pull_none>;
		};
	};
......
};

Then recompile the dtb and upload it to the board to overwrite the old one. After booting, I confirmed that the changes took effect.

fdtdump /sys/firmware/fdt | grep -A 2 'dp-hpd' output：

**** fdtdump is a low-level debugging tool, not meant for general use.
**** If you want to decompile a dtb, you probably want
****     dtc -I dtb -O dts <filename>

            edp-hpd {
                rockchip,pins = <0x00000004 0x00000017 0x00000002 0x000000b4>;
                phandle = <0x000000af>;
--
            dp-hpd {
                rockchip,pins = <0x00000004 0x00000019 0x00000000 0x000000b9>;
            };

fdtdump /sys/firmware/fdt | grep -B 3 'phandle = <0x000000b9>' output：

**** fdtdump is a low-level debugging tool, not meant for general use.
**** If you want to decompile a dtb, you probably want
****     dtc -I dtb -O dts <filename>

        pcfg-input-pull-up {
            input-enable;
            bias-pull-up;
            phandle = <0x000000b9>;

You can see that dp-hpd is indeed configured as input and pulled up. At this time, I changed the gpio request function in the driver back to GPIOD_OUT_LOW, recompiled and loaded it, but it still failed:

[   79.202043] extcon_usbc_virtual_pd2: loading out-of-tree module taints kernel.
[   79.208260] gpio gpiochip4: (gpio4): gpiochip_lock_as_irq: tried to flag a GPIO set as output for IRQ
[   79.209160] gpio gpiochip4: (gpio4): unable to lock HW IRQ 25 for IRQ
[   79.209771] genirq: Failed to request resources for virtual-pd (irq 88) on irqchip rockchip_gpio_irq
[   79.211069] extcon-usbc-virtual-pd virtual-pd: failed to request gpio irq
[   79.211726] extcon-usbc-virtual-pd virtual-pd: probe with driver extcon-usbc-virtual-pd failed with error -5

Change GPIOD_OUT_LOW in the driver to GPIOD_IN and it will return to normal.

According to the error message, I guess that although the default initial state of GPIO is set in dts, vpd->det_gpio = devm_gpiod_get_optional(dev, "det", GPIOD_OUT_LOW); in the driver changes it to output, followed by vpd->irq = gpiod_to_irq(vpd->det_gpio);Apply irq for it, but applying for interrupt for a GPIO with output status is not allowed? That’s why there is gpiochip_lock_as_irq failed in dmesg

Regarding the second point, your concerns are correct, but unfortunately I do not have the Rockpi 4C held by the patch author (the DT binding document example of this patch uses SBC). I don’t know whether my modification will destroy the original HDMI function, but searching for the words extcon-usbc-virtual-pd in the Armbian build repository, no other devices are using it.

[paolosabatino]

Did you try with GPIOD_ASIS flag in place of other GPIOD_* flags to keep the default stated by pinctrl device property?

Now I had the chance to verify in the armbian codebase, and it looks to me that no board is actually using this driver.
I guess you can feel free to fix it straight away with GPIOD_INPUT or implement a more proper fix with pinctrl and GPIOD_ASIS (if it works), just tell me what you prefer and when you're ready and I will accept the PR! 😉

[retro98boy]

Did you try with GPIOD_ASIS flag in place of other GPIOD_* flags to keep the default stated by pinctrl device property?

Now I had the chance to verify in the armbian codebase, and it looks to me that no board is actually using this driver. I guess you can feel free to fix it straight away with GPIOD_INPUT or implement a more proper fix with pinctrl and GPIOD_ASIS (if it works), just tell me what you prefer and when you're ready and I will accept the PR! 😉

I set the GPIO's pinctrl as input in dts, then requested it using the GPIOD_ASIS flag in the driver, and it worked! I'll modify the PR