| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1a1f6b09-11cc-fa38-ac32-aa0f8865aaea@arm.com>
Date: Wed, 1 May 2019 15:52:37 +0100
From: Robin Murphy <robin.murphy@....com>
To: Jagan Teki <jagan@...rulasolutions.com>
Cc: Heiko Stuebner <heiko@...ech.de>, Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
devicetree <devicetree@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-rockchip@...ts.infradead.org,
linux-amarula <linux-amarula@...rulasolutions.com>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [DO NOT MERGE] [PATCH 2/2] arm64: rockchip: rk3399: nanopc-t4:
Enable FriendlyELEC HD702E eDP panel
On 01/05/2019 15:09, Jagan Teki wrote:
> On Wed, May 1, 2019 at 6:17 PM Robin Murphy <robin.murphy@....com> wrote:
>>
>> On 01/05/2019 13:14, Jagan Teki wrote:
>>> FriendlyELEC HD702E is one of optional LCD panel for
>>> NanoPC T4 eDP interface.
>>>
>>> It features 800x1280 resolutions, with built in GT9271 captive
>>> touchscreen and adjustable backlight via PWM.
>>>
>>> eDP panel connections are:
>>> - VCC3V3_SYS: 3.3V panel power supply
>>> - GPIO4_C2: PWM0_BL pin
>>> - GPIO4_D5_LCD_BL_EN: Backlight enable pin
>>> - VCC12V0_SYS: 12V backlight power supply
>>> - Touchscreen connected via I2C4
>>> - GPIO1_C4_TP_INT: touchscreen interrupt pin
>>> - GPIO1_B5_TP_RST: touchscreen reset pin
>>>
>>> Add support for it.
>>>
>>> Signed-off-by: Jagan Teki <jagan@...rulasolutions.com>
>>> ---
>>> Note: we need to disable hdmi-cec pinctrl to work with
>>> edp-hpd since both share same pin, otherwise we can
>>> encounter below error during bootup
>>> [ 1.047726] rockchip-pinctrl pinctrl: pin gpio4-23 already requested by ff940000.hdmi; cannot claim for ff970000.edp
>>> [ 1.048655] rockchip-pinctrl pinctrl: pin-151 (ff970000.edp) status -22
>>> [ 1.049235] rockchip-pinctrl pinctrl: could not request pin 151 (gpio4-23) from group edp-hpd on device rockchip-pinctrl
>>> [ 1.050191] rockchip-dp ff970000.edp: Error applying setting, reverse things back
>>> [ 1.050867] rockchip-dp: probe of ff970000.edp failed with error -22
>>
>> Hmm, AFAICS that pin is exclusively wired to the HDMI connector and not
>> used for the eDP interface, so really it's the fault of rk3399.dtsi for
>> trying to claim it unconditionally. Ideally we'd pull those pinctrl
>> properties out into the board DTs which do actually need them, but the
>> quick and easy approach would be to add some "/delete-property/ ..."
>> workarounds to the &edp node here.
>
> Thought that initially, but the same pin shared between HDMI CEC and
> eDP hotplug with different bit function to enable.
>
> gpio4c7_sel
> GPIO4C[7] iomux select
> 2'b00: gpio
> 2'b01: hdmi_cecinout
> 2'b10: edp_hotplug
> 2'b11: reserved
>
> GPIO4_C7/HDMI_CECINOUT/EDP_HOTPLUG is the shared pin, which is
> available in any nanopc-t4 as well in rk3399 datasheet, look like it's
> an SoC pin that driver hotplug to eDP and ie same reason is pinmux in
> rk3399.dtsi.
>
> I event removed edp_hpd pinctrl from edp node in rk3399.dtsi, but
> display not appear on the screen and observed edp bridge issue on
> host.
Ah, I see - from a quick scan through the drivers it looks the
"force-hpd" property already exists as the solution for that. Either way
I don't think it would be safe to rely on the EDP_HOTPLUG pinctrl
appearing to work, given that the physical pin is hard-wired to the HDMI
connector - presumably if you tried to use the eDP display while a TV is
plugged in and periodically sending CEC messages, all kinds of
shenanigans might ensue.
> [ 1.052191] rockchip-drm display-subsystem: bound ff8f0000.vop (ops
> vop_component_ops)
> [ 1.054460] rockchip-drm display-subsystem: bound ff900000.vop (ops
> vop_component_ops)
> [ 1.055214] rockchip-dp ff970000.edp: no DP phy configured
> [ 1.056088] rockchip-drm display-subsystem: bound ff970000.edp (ops
> rockchip_dp_component_ops)
> [ 1.056852] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [ 1.057449] [drm] No driver support for vblank timestamp query.
> [ 1.174379] [drm:analogix_dp_bridge_enable] *ERROR* failed to get
> hpd single ret = -110
> [ 1.174408] rockchip-dp ff970000.edp: failed to set bridge, retry: 0
> [ 1.285524] [drm:analogix_dp_bridge_enable] *ERROR* failed to get
> hpd single ret = -110
> [ 1.285539] rockchip-dp ff970000.edp: failed to set bridge, retry: 1
> [ 1.355241] dwmmc_rockchip fe310000.dwmmc: Successfully tuned phase to 212
> [ 1.358757] mmc0: new ultra high speed SDR104 SDIO card at address 0001
> [ 1.397049] [drm:analogix_dp_bridge_enable] *ERROR* failed to get
> hpd single ret = -110
> [ 1.397069] rockchip-dp ff970000.edp: failed to set bridge, retry: 2
> [ 1.485582] dwmmc_rockchip fe320000.dwmmc: Successfully tuned phase to 220
> [ 1.485590] mmc1: new ultra high speed SDR104 SDHC card at address 084e
> [ 1.486246] mmcblk1: mmc1:084e R04GS 3.71 GiB
> [ 1.488032] mmcblk1: p1
> [ 1.509088] [drm:analogix_dp_bridge_enable] *ERROR* failed to get
> hpd single ret = -110
> [ 1.509119] rockchip-dp ff970000.edp: failed to set bridge, retry: 3
> [ 1.620938] [drm:analogix_dp_bridge_enable] *ERROR* failed to get
> hpd single ret = -110
> [ 1.620953] rockchip-dp ff970000.edp: failed to set bridge, retry: 4
> [ 1.620970] rockchip-dp ff970000.edp: too many times retry set
> bridge, give it up
> [ 1.644026] Console: switching to colour frame buffer device 100x80
>
>
>>
>>> .../boot/dts/rockchip/rk3399-nanopc-t4.dts | 82 +++++++++++++++++++
>>> 1 file changed, 82 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts b/arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts
>>> index 931c3dbf1b7d..b652d960946f 100644
>>> --- a/arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts
>>> +++ b/arch/arm64/boot/dts/rockchip/rk3399-nanopc-t4.dts
>>> @@ -46,6 +46,48 @@
>>> };
>>> };
>>>
>>> + backlight: backlight {
>>> + compatible = "pwm-backlight";
>>> + brightness-levels = <
>>> + 0 1 2 3 4 5 6 7
>>> + 8 9 10 11 12 13 14 15
>>> + 16 17 18 19 20 21 22 23
>>> + 24 25 26 27 28 29 30 31
>>> + 32 33 34 35 36 37 38 39
>>> + 40 41 42 43 44 45 46 47
>>> + 48 49 50 51 52 53 54 55
>>> + 56 57 58 59 60 61 62 63
>>> + 64 65 66 67 68 69 70 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 100 101 102 103
>>> + 104 105 106 107 108 109 110 111
>>> + 112 113 114 115 116 117 118 119
>>> + 120 121 122 123 124 125 126 127
>>> + 128 129 130 131 132 133 134 135
>>> + 136 137 138 139 140 141 142 143
>>> + 144 145 146 147 148 149 150 151
>>> + 152 153 154 155 156 157 158 159
>>> + 160 161 162 163 164 165 166 167
>>> + 168 169 170 171 172 173 174 175
>>> + 176 177 178 179 180 181 182 183
>>> + 184 185 186 187 188 189 190 191
>>> + 192 193 194 195 196 197 198 199
>>> + 200 201 202 203 204 205 206 207
>>> + 208 209 210 211 212 213 214 215
>>> + 216 217 218 219 220 221 222 223
>>> + 224 225 226 227 228 229 230 231
>>> + 232 233 234 235 236 237 238 239
>>> + 240 241 242 243 244 245 246 247
>>> + 248 249 250 251 252 253 254 255>;
>>
>> This looks trivial enough that I wonder whether it might still work to
>> just omit it? Not that I know anything about backlights, but I had the
>> impression (from mailing list traffic, I guess) that the driver gained
>> the ability to provide a reasonable default behaviour at some point.
>
> Unaware about this, would you please pass the thread. on the
> other-hand I can see sapphire-excavator still using the brightness
> levels like this.
It looks like the "default behaviour" thing must have been what ended up
as commit 88ba95bedb79 ("backlight: pwm_bl: Compute brightness of LED
linearly to human eye"). Even if that turns out to be unsuitable for
this particular panel, it seems that the PWM-linear response could now
be specified much more simply as something like:
brightness-levels = <0 255>;
num-interpolated-steps = <256>;
Robin.
Powered by blists - more mailing lists