On Tue, 1 Apr 2025 at 00:38, Christopher Obbard <[email protected]> wrote: > > Hi Dmitry, > > On Mon, 31 Mar 2025 at 17:49, Dmitry Baryshkov > <[email protected]> wrote: > > > > On 31/03/2025 18:39, Christopher Obbard wrote: > > > Hi Johan, > > > > > > On Mon, 31 Mar 2025 at 09:50, Johan Hovold <[email protected]> wrote: > > >> > > >> On Thu, Mar 27, 2025 at 04:56:53PM +0000, Christopher Obbard wrote: > > >>> The eDP panel has an HPD GPIO. Describe it in the device tree > > >>> for the generic T14s model, as the HPD GPIO property is used in > > >>> both the OLED and LCD models which inherit this device tree. > > >> > > >> AFAICT, this patch is not correct as the hotplug detect signal is > > >> connected directly to the display controller on (these) Qualcomm SoCs > > >> and is already handled by its driver. > > >> > > >> Describing it as you do here leads to less accurate delays, see commits: > > >> > > >> 2327b13d6c47 ("drm/panel-edp: Take advantage of > > >> wait_hpd_asserted() in struct drm_dp_aux"). > > >> 3b5765df375c ("drm/panel: atna33xc20: Take advantage of > > >> wait_hpd_asserted() in struct drm_dp_aux") > > >> > > >> Perhaps you lose some other functionality too. > > >> > > >>> Reviewed-by: Bryan O'Donoghue <[email protected]> > > >>> Signed-off-by: Christopher Obbard <[email protected]> > > >>> --- > > >>> arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi | 11 > > >>> +++++++++++ > > >>> 1 file changed, 11 insertions(+) > > >>> > > >>> diff --git > > >>> a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi > > >>> b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi > > >>> index > > >>> 962fb050c55c4fd33f480a21a8c47a484d0c82b8..46c73f5c039ed982b553636cf8c4237a20ba7687 > > >>> 100644 > > >>> --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi > > >>> +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi > > >>> @@ -980,8 +980,12 @@ &mdss_dp3 { > > >>> aux-bus { > > >>> panel: panel { > > >>> compatible = "edp-panel"; > > >>> + hpd-gpios = <&tlmm 119 GPIO_ACTIVE_HIGH>; > > >>> power-supply = <&vreg_edp_3p3>; > > >>> > > >>> + pinctrl-0 = <&edp_hpd_n_default>; > > >>> + pinctrl-names = "default"; > > >>> + > > >>> port { > > >>> edp_panel_in: endpoint { > > >>> remote-endpoint = > > >>> <&mdss_dp3_out>; > > >>> @@ -1286,6 +1290,13 @@ hall_int_n_default: hall-int-n-state { > > >>> bias-disable; > > >>> }; > > >>> > > >>> + edp_hpd_n_default: edp-hpd-n-state { > > >>> + pins = "gpio119"; > > >>> + function = "gpio"; > > >>> + drive-strength = <2>; > > >>> + bias-pull-up; > > >>> + }; > > >> > > >> I checked the firmware configuration for this pin on my T14s, which > > >> does not match what you have here. Instead the function is set to > > >> "edp0_hot" which forwards the signal to the display controller which > > >> already handles the signal on panel power on. (And there is also no > > >> internal pull up enabled). > > >> > > >> We may want to describe this pin configuration somewhere, but that's a > > >> separate issue. > > > > > > Thanks for your review, I will send another version in coming days and > > > drop this first patch (adding hpd to the T14s DTSI). > > > > > > As a consequence I will need to add no-hpd property to the panel node. > > No, you won't. There is a HPD line and it is routed to the DP controller. > > OK, I think I misunderstand what Johan said. After taking some time to > think about it in more detail: > - The first commit will be changed so that the hpd GPIO will be added > to the X1E DP controller instead of the panel. grepping the source for > dp_hot_plug_det shows me how to do that. This part is clear. > > - The panel node in the generic T14s DTSI should not have the > hpd-gpios property / pinctrl set. > > - The panel node should not have the hpd-gpios property / pinctrl set.
LGTM. Use sc8180x-primus as an example > > I hope I understand that correctly. I will send a new series in the > morning unless there is any objection. -- With best wishes Dmitry
