Message ID | 20230404104800.301150-1-angelogioacchino.delregno@collabora.com |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp2927602vqo; Tue, 4 Apr 2023 03:49:06 -0700 (PDT) X-Google-Smtp-Source: AKy350YcibvNRcdmNLydoO3sCXU3Gcj9YpjbrrgANYkNhqg+e+AJoS55yOqRW95F068DzlThe4Bo X-Received: by 2002:a17:906:858f:b0:90b:167e:304b with SMTP id v15-20020a170906858f00b0090b167e304bmr2059244ejx.45.1680605345806; Tue, 04 Apr 2023 03:49:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680605345; cv=none; d=google.com; s=arc-20160816; b=aidKLbA3gzspaMULAdPr/tUhvu/BECRDRRwyr9JdUTkkGpwy878TB0cvfTl5yXCmCC XYSNvmEvT85/jP6b7G6ASbMwZPXsjG/ZHN5+UByRXQCetRe0xE12Nj5aO2UxL2obCALn yEoFcYS+ro6oBxl31ooNOv8VbfTTA0OAlZdK4ttXtgm2xQBabq/ilrIDUUFaggioCInO biri/IYdemhIXsmZ13T7gXclqSvGANWfUBaIhveUb9KFOvNi7pnshRo6VPmjAsKJy/qM H06+TWlPX1YhS+tV/OwqpXhDPCOpHhdAMXVuIiMvzVDQiFtsN+33q2G/sqlHrw+t2ss7 Q/eQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=EXO06mS+aWHlC2y5W26USHjFCj57Z3C3TUby2nJzq30=; b=gbEAB24b+BKqw2Y+b1V3T2Nnllb0Nl9F3djhF60Yrdc2JWRKrHFesgsdI+UJGm+xAq DnUXNoyAx2OlOQgAZbNSQSCqILKj9/ey39W2ftEJtEhzFtKYXYCmmIh7zTgvOsVHd8Pf mJ1gFHk7NU4FII62rC4p7UM3oAzPZcTUneWcVATGKJJBmF7NA5FowWU2ZX86tp1H1Z0L CYqtuSazOHtYHVXYJHowFp0QbuOzV4/q9DXUOy7Yyxk4k4HddAp4cghMqdC1RSBYFgWX wKUAKYD8k7Ba9Z2dw134Q2CDnHh3Gelue4vFYYQ91EwnZ/IaizxoewnSnrUkMgl1rJ9Q sKmw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=CXHh+puI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id f10-20020a170906560a00b008d4d102a7b8si1405396ejq.365.2023.04.04.03.48.41; Tue, 04 Apr 2023 03:49:05 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=CXHh+puI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234461AbjDDKsL (ORCPT <rfc822;lkml4gm@gmail.com> + 99 others); Tue, 4 Apr 2023 06:48:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57044 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233904AbjDDKsJ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 4 Apr 2023 06:48:09 -0400 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 91D491981 for <linux-kernel@vger.kernel.org>; Tue, 4 Apr 2023 03:48:07 -0700 (PDT) Received: from IcarusMOD.eternityproject.eu (2-237-20-237.ip236.fastwebnet.it [2.237.20.237]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id 65B3B660000E; Tue, 4 Apr 2023 11:48:05 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1680605286; bh=AAPNaHX0V1PialVkpX+4zJQpYY6tpjp8IWS8ZSYvSmQ=; h=From:To:Cc:Subject:Date:From; b=CXHh+puIkl5QU4tQoD0D1dPauiD9KWhgyP1q9dOW/XApTnvtC/AYSM7nmdrRBGvZi BSJgZEg4YbcVcvloFl2tg418BQq6B79yqeV6msXTk9Psz5SCrzvswQ2RVplfeTHc29 g3Xjznd2yVN6Nj8TfftDGeEwR3z0B7A96+rKxCihiFW5r1rDKYO+ZNZJktvcqfOrJy M7iMgCrRIYghjXYq/X4rPjA47xEUihUQZHzb4tERhl9AqOFDqraWqcv6h/T6R+e0N6 vzi5G4kySbHyWYn6Ts7IK4d2wQosAprUDTVeI/27hgl9tEYDcHSQMwc0x67DRO1joY IdWzYbjR9K6TQ== From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> To: chunkuang.hu@kernel.org Cc: p.zabel@pengutronix.de, airlied@gmail.com, daniel@ffwll.ch, matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com, dri-devel@lists.freedesktop.org, linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel@collabora.com, wenst@chromium.org Subject: [PATCH v3 0/9] MediaTek DisplayPort: support eDP and aux-bus Date: Tue, 4 Apr 2023 12:47:51 +0200 Message-Id: <20230404104800.301150-1-angelogioacchino.delregno@collabora.com> X-Mailer: git-send-email 2.40.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1762242430908746386?= X-GMAIL-MSGID: =?utf-8?q?1762242430908746386?= |
Series |
MediaTek DisplayPort: support eDP and aux-bus
|
|
Message
AngeloGioacchino Del Regno
April 4, 2023, 10:47 a.m. UTC
Changes in v3: - Added DPTX AUX block initialization before trying to communicate to stop relying on the bootloader keeping it initialized before booting Linux. - Fixed commit description for patch [09/09] and removed commented out code (that slipped from dev phase.. sorry!). This series adds "real" support for eDP in the mtk-dp DisplayPort driver. Explaining the "real": Before this change, the DisplayPort driver did support eDP to some extent, but it was treating it entirely like a regular DP interface which is partially fine, after all, embedded DisplayPort *is* actually DisplayPort, but there might be some differences to account for... and this is for both small performance improvements and, more importantly, for correct functionality in some systems. Functionality first: One of the common differences found in various boards implementing eDP and machines using an eDP panel is that many times the HPD line is not connected. This *must* be accounted for: at startup, this specific IP will raise a HPD interrupt (which should maybe be ignored... as it does not appear to be a "real" event...) that will make the eDP panel to be detected and to actually work but, after a suspend-resume cycle, there will be no HPD interrupt (as there's no HPD line in my case!) producing a functionality issue - specifically, the DP Link Training fails because the panel doesn't get powered up, then it stays black and won't work until rebooting the machine (or removing and reinserting the module I think, but I haven't tried that). Now for.. both: eDP panels are *e*DP because they are *not* removable (in the sense that you can't unplug the cable without disassembling the machine, in which case, the machine shall be powered down..!): this (correct) assumption makes us able to solve some issues and to also gain a little performance during PM operations. What was done here is: - Caching the EDID if the panel is eDP: we're always going to read the same data everytime, so we can just cache that (as it's small enough) shortening PM resume times for the eDP driver instance; - Always return connector_status_connected if it's eDP: non-removable means connector_status_disconnected can't happen during runtime... this also saves us some time and even power, as we won't have to perform yet another power cycle of the HW; - Added aux-bus support! This makes us able to rely on panel autodetection from the EDID, avoiding to add more and more panel timings to panel-edp and, even better, allowing to use one panel node in devicetrees for multiple variants of the same machine since, at that point, it's not important to "preventively know" what panel we have (eh, it's autodetected...!). This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus) P.S.: For your own testing commodity, here's a reference devicetree: &edp_tx { status = "okay"; pinctrl-names = "default"; pinctrl-0 = <&edptx_pins_default>; ports { #address-cells = <1>; #size-cells = <0>; port@0 { reg = <0>; edp_in: endpoint { remote-endpoint = <&dp_intf0_out>; }; }; port@1 { reg = <1>; edp_out: endpoint { data-lanes = <0 1 2 3>; remote-endpoint = <&panel_in>; }; }; }; aux-bus { panel: panel { compatible = "edp-panel"; power-supply = <&pp3300_disp_x>; backlight = <&backlight_lcd0>; port { panel_in: endpoint { remote-endpoint = <&edp_out>; }; }; }; }; }; AngeloGioacchino Del Regno (9): drm/mediatek: dp: Cache EDID for eDP panel drm/mediatek: dp: Move AUX and panel poweron/off sequence to function drm/mediatek: dp: Always return connected status for eDP in .detect() drm/mediatek: dp: Always set cable_plugged_in at resume for eDP panel drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() drm/mediatek: dp: Enable event interrupt only when bridge attached drm/mediatek: dp: Use devm variant of drm_bridge_add() drm/mediatek: dp: Move AUX_P0 setting to mtk_dp_initialize_aux_settings() drm/mediatek: dp: Add support for embedded DisplayPort aux-bus drivers/gpu/drm/mediatek/mtk_dp.c | 186 +++++++++++++++++++----------- 1 file changed, 116 insertions(+), 70 deletions(-)
Comments
On Tue, Apr 4, 2023 at 6:48 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Changes in v3: > - Added DPTX AUX block initialization before trying to communicate > to stop relying on the bootloader keeping it initialized before > booting Linux. > - Fixed commit description for patch [09/09] and removed commented > out code (that slipped from dev phase.. sorry!). > > This series adds "real" support for eDP in the mtk-dp DisplayPort driver. > > Explaining the "real": > Before this change, the DisplayPort driver did support eDP to some > extent, but it was treating it entirely like a regular DP interface > which is partially fine, after all, embedded DisplayPort *is* actually > DisplayPort, but there might be some differences to account for... and > this is for both small performance improvements and, more importantly, > for correct functionality in some systems. > > Functionality first: > > One of the common differences found in various boards implementing eDP > and machines using an eDP panel is that many times the HPD line is not > connected. This *must* be accounted for: at startup, this specific IP > will raise a HPD interrupt (which should maybe be ignored... as it does > not appear to be a "real" event...) that will make the eDP panel to be > detected and to actually work but, after a suspend-resume cycle, there > will be no HPD interrupt (as there's no HPD line in my case!) producing > a functionality issue - specifically, the DP Link Training fails because > the panel doesn't get powered up, then it stays black and won't work > until rebooting the machine (or removing and reinserting the module I > think, but I haven't tried that). > > Now for.. both: > eDP panels are *e*DP because they are *not* removable (in the sense that > you can't unplug the cable without disassembling the machine, in which > case, the machine shall be powered down..!): this (correct) assumption > makes us able to solve some issues and to also gain a little performance > during PM operations. > > What was done here is: > - Caching the EDID if the panel is eDP: we're always going to read the > same data everytime, so we can just cache that (as it's small enough) > shortening PM resume times for the eDP driver instance; > - Always return connector_status_connected if it's eDP: non-removable > means connector_status_disconnected can't happen during runtime... > this also saves us some time and even power, as we won't have to > perform yet another power cycle of the HW; > - Added aux-bus support! > This makes us able to rely on panel autodetection from the EDID, > avoiding to add more and more panel timings to panel-edp and, even > better, allowing to use one panel node in devicetrees for multiple > variants of the same machine since, at that point, it's not important > to "preventively know" what panel we have (eh, it's autodetected...!). > > This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus) > > > P.S.: For your own testing commodity, here's a reference devicetree: > &edp_tx { > status = "okay"; > > pinctrl-names = "default"; > pinctrl-0 = <&edptx_pins_default>; > > ports { > #address-cells = <1>; > #size-cells = <0>; > > port@0 { > reg = <0>; > edp_in: endpoint { > remote-endpoint = <&dp_intf0_out>; > }; > }; > > port@1 { > reg = <1>; > edp_out: endpoint { > data-lanes = <0 1 2 3>; > remote-endpoint = <&panel_in>; > }; > }; > }; > > aux-bus { > panel: panel { > compatible = "edp-panel"; > power-supply = <&pp3300_disp_x>; > backlight = <&backlight_lcd0>; > port { > panel_in: endpoint { > remote-endpoint = <&edp_out>; > }; > }; > }; > }; > }; > > AngeloGioacchino Del Regno (9): > drm/mediatek: dp: Cache EDID for eDP panel > drm/mediatek: dp: Move AUX and panel poweron/off sequence to function > drm/mediatek: dp: Always return connected status for eDP in .detect() > drm/mediatek: dp: Always set cable_plugged_in at resume for eDP panel > drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() > drm/mediatek: dp: Enable event interrupt only when bridge attached > drm/mediatek: dp: Use devm variant of drm_bridge_add() > drm/mediatek: dp: Move AUX_P0 setting to > mtk_dp_initialize_aux_settings() > drm/mediatek: dp: Add support for embedded DisplayPort aux-bus Tested-by: Chen-Yu Tsai <wenst@chromium.org> on MT8195 Tomato: eDP panel works if the display panel regulator is always on. External DP works. Maybe it has something to do with the driver not supporting .wait_hpd_asserted and not using a GPIO for HPD?
Il 06/04/23 09:20, Chen-Yu Tsai ha scritto: > On Tue, Apr 4, 2023 at 6:48 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Changes in v3: >> - Added DPTX AUX block initialization before trying to communicate >> to stop relying on the bootloader keeping it initialized before >> booting Linux. >> - Fixed commit description for patch [09/09] and removed commented >> out code (that slipped from dev phase.. sorry!). >> >> This series adds "real" support for eDP in the mtk-dp DisplayPort driver. >> >> Explaining the "real": >> Before this change, the DisplayPort driver did support eDP to some >> extent, but it was treating it entirely like a regular DP interface >> which is partially fine, after all, embedded DisplayPort *is* actually >> DisplayPort, but there might be some differences to account for... and >> this is for both small performance improvements and, more importantly, >> for correct functionality in some systems. >> >> Functionality first: >> >> One of the common differences found in various boards implementing eDP >> and machines using an eDP panel is that many times the HPD line is not >> connected. This *must* be accounted for: at startup, this specific IP >> will raise a HPD interrupt (which should maybe be ignored... as it does >> not appear to be a "real" event...) that will make the eDP panel to be >> detected and to actually work but, after a suspend-resume cycle, there >> will be no HPD interrupt (as there's no HPD line in my case!) producing >> a functionality issue - specifically, the DP Link Training fails because >> the panel doesn't get powered up, then it stays black and won't work >> until rebooting the machine (or removing and reinserting the module I >> think, but I haven't tried that). >> >> Now for.. both: >> eDP panels are *e*DP because they are *not* removable (in the sense that >> you can't unplug the cable without disassembling the machine, in which >> case, the machine shall be powered down..!): this (correct) assumption >> makes us able to solve some issues and to also gain a little performance >> during PM operations. >> >> What was done here is: >> - Caching the EDID if the panel is eDP: we're always going to read the >> same data everytime, so we can just cache that (as it's small enough) >> shortening PM resume times for the eDP driver instance; >> - Always return connector_status_connected if it's eDP: non-removable >> means connector_status_disconnected can't happen during runtime... >> this also saves us some time and even power, as we won't have to >> perform yet another power cycle of the HW; >> - Added aux-bus support! >> This makes us able to rely on panel autodetection from the EDID, >> avoiding to add more and more panel timings to panel-edp and, even >> better, allowing to use one panel node in devicetrees for multiple >> variants of the same machine since, at that point, it's not important >> to "preventively know" what panel we have (eh, it's autodetected...!). >> >> This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus) >> >> >> P.S.: For your own testing commodity, here's a reference devicetree: >> &edp_tx { >> status = "okay"; >> >> pinctrl-names = "default"; >> pinctrl-0 = <&edptx_pins_default>; >> >> ports { >> #address-cells = <1>; >> #size-cells = <0>; >> >> port@0 { >> reg = <0>; >> edp_in: endpoint { >> remote-endpoint = <&dp_intf0_out>; >> }; >> }; >> >> port@1 { >> reg = <1>; >> edp_out: endpoint { >> data-lanes = <0 1 2 3>; >> remote-endpoint = <&panel_in>; >> }; >> }; >> }; >> >> aux-bus { >> panel: panel { >> compatible = "edp-panel"; >> power-supply = <&pp3300_disp_x>; >> backlight = <&backlight_lcd0>; >> port { >> panel_in: endpoint { >> remote-endpoint = <&edp_out>; >> }; >> }; >> }; >> }; >> }; >> >> AngeloGioacchino Del Regno (9): >> drm/mediatek: dp: Cache EDID for eDP panel >> drm/mediatek: dp: Move AUX and panel poweron/off sequence to function >> drm/mediatek: dp: Always return connected status for eDP in .detect() >> drm/mediatek: dp: Always set cable_plugged_in at resume for eDP panel >> drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() >> drm/mediatek: dp: Enable event interrupt only when bridge attached >> drm/mediatek: dp: Use devm variant of drm_bridge_add() >> drm/mediatek: dp: Move AUX_P0 setting to >> mtk_dp_initialize_aux_settings() >> drm/mediatek: dp: Add support for embedded DisplayPort aux-bus > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > > on MT8195 Tomato: eDP panel works if the display panel regulator is always > on. External DP works. > > Maybe it has something to do with the driver not supporting .wait_hpd_asserted > and not using a GPIO for HPD? Even before this change I couldn't get the panel to reliably work without keeping the regulator always on (just to point out that I'm not introducing regressions). I am already trying to understand why this happens... and I'm still researching... but there's what I've seen for now: * Set the panel regulator as regulator-boot-on; * Boot: edp-panel will correctly read the EDID, then will run the PM suspend handler * mtk-dp's .get_edid() callback gets called but, at that time, edp-panel will still be suspended (PM resume handler didn't get called) * Regulator is still down * Failure. That's not right and probably the .get_edid() callback in mtk-dp has an abuse: there, mtk_dp_parse_capabilities() gets called, which performs initialization of some variables for DP link training (essential to get the DP going!). The question that I am making to myself is whether I should move that elsewhere, if so, what's the right place (making me able to remove the DRM_BRIDGE_OP_EDID bridge flag when eDP + aux-bus), or if I should leave that and make sure that the panel-edp's resume callback is called before .get_edid() from mtk-dp gets called. That can get done in a separated series (or single patch?)... so that if we get this one picked sooner than later, we can start upstreaming the panel nodes in the Cherry devicetrees and only remove the regulator-always-on later. Thoughts? Cheers, Angelo
On Thu, Apr 6, 2023 at 4:25 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 06/04/23 09:20, Chen-Yu Tsai ha scritto: > > On Tue, Apr 4, 2023 at 6:48 PM AngeloGioacchino Del Regno > > <angelogioacchino.delregno@collabora.com> wrote: > >> > >> Changes in v3: > >> - Added DPTX AUX block initialization before trying to communicate > >> to stop relying on the bootloader keeping it initialized before > >> booting Linux. > >> - Fixed commit description for patch [09/09] and removed commented > >> out code (that slipped from dev phase.. sorry!). > >> > >> This series adds "real" support for eDP in the mtk-dp DisplayPort driver. > >> > >> Explaining the "real": > >> Before this change, the DisplayPort driver did support eDP to some > >> extent, but it was treating it entirely like a regular DP interface > >> which is partially fine, after all, embedded DisplayPort *is* actually > >> DisplayPort, but there might be some differences to account for... and > >> this is for both small performance improvements and, more importantly, > >> for correct functionality in some systems. > >> > >> Functionality first: > >> > >> One of the common differences found in various boards implementing eDP > >> and machines using an eDP panel is that many times the HPD line is not > >> connected. This *must* be accounted for: at startup, this specific IP > >> will raise a HPD interrupt (which should maybe be ignored... as it does > >> not appear to be a "real" event...) that will make the eDP panel to be > >> detected and to actually work but, after a suspend-resume cycle, there > >> will be no HPD interrupt (as there's no HPD line in my case!) producing > >> a functionality issue - specifically, the DP Link Training fails because > >> the panel doesn't get powered up, then it stays black and won't work > >> until rebooting the machine (or removing and reinserting the module I > >> think, but I haven't tried that). > >> > >> Now for.. both: > >> eDP panels are *e*DP because they are *not* removable (in the sense that > >> you can't unplug the cable without disassembling the machine, in which > >> case, the machine shall be powered down..!): this (correct) assumption > >> makes us able to solve some issues and to also gain a little performance > >> during PM operations. > >> > >> What was done here is: > >> - Caching the EDID if the panel is eDP: we're always going to read the > >> same data everytime, so we can just cache that (as it's small enough) > >> shortening PM resume times for the eDP driver instance; > >> - Always return connector_status_connected if it's eDP: non-removable > >> means connector_status_disconnected can't happen during runtime... > >> this also saves us some time and even power, as we won't have to > >> perform yet another power cycle of the HW; > >> - Added aux-bus support! > >> This makes us able to rely on panel autodetection from the EDID, > >> avoiding to add more and more panel timings to panel-edp and, even > >> better, allowing to use one panel node in devicetrees for multiple > >> variants of the same machine since, at that point, it's not important > >> to "preventively know" what panel we have (eh, it's autodetected...!). > >> > >> This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus) > >> > >> > >> P.S.: For your own testing commodity, here's a reference devicetree: > >> &edp_tx { > >> status = "okay"; > >> > >> pinctrl-names = "default"; > >> pinctrl-0 = <&edptx_pins_default>; > >> > >> ports { > >> #address-cells = <1>; > >> #size-cells = <0>; > >> > >> port@0 { > >> reg = <0>; > >> edp_in: endpoint { > >> remote-endpoint = <&dp_intf0_out>; > >> }; > >> }; > >> > >> port@1 { > >> reg = <1>; > >> edp_out: endpoint { > >> data-lanes = <0 1 2 3>; > >> remote-endpoint = <&panel_in>; > >> }; > >> }; > >> }; > >> > >> aux-bus { > >> panel: panel { > >> compatible = "edp-panel"; > >> power-supply = <&pp3300_disp_x>; > >> backlight = <&backlight_lcd0>; > >> port { > >> panel_in: endpoint { > >> remote-endpoint = <&edp_out>; > >> }; > >> }; > >> }; > >> }; > >> }; > >> > >> AngeloGioacchino Del Regno (9): > >> drm/mediatek: dp: Cache EDID for eDP panel > >> drm/mediatek: dp: Move AUX and panel poweron/off sequence to function > >> drm/mediatek: dp: Always return connected status for eDP in .detect() > >> drm/mediatek: dp: Always set cable_plugged_in at resume for eDP panel > >> drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() > >> drm/mediatek: dp: Enable event interrupt only when bridge attached > >> drm/mediatek: dp: Use devm variant of drm_bridge_add() > >> drm/mediatek: dp: Move AUX_P0 setting to > >> mtk_dp_initialize_aux_settings() > >> drm/mediatek: dp: Add support for embedded DisplayPort aux-bus > > > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > > > > on MT8195 Tomato: eDP panel works if the display panel regulator is always > > on. External DP works. > > > > Maybe it has something to do with the driver not supporting .wait_hpd_asserted > > and not using a GPIO for HPD? > > Even before this change I couldn't get the panel to reliably work without keeping > the regulator always on (just to point out that I'm not introducing regressions). > > I am already trying to understand why this happens... and I'm still researching... > but there's what I've seen for now: > * Set the panel regulator as regulator-boot-on; This simply means when the regulator driver grabs the GPIO, it will grab it with output high. It will make no attempt to keep it on. > * Boot: edp-panel will correctly read the EDID, then will run the PM suspend > handler > * mtk-dp's .get_edid() callback gets called but, at that time, edp-panel will > still be suspended (PM resume handler didn't get called) > * Regulator is still down > * Failure. > That's not right and probably the .get_edid() callback in mtk-dp has an abuse: > there, mtk_dp_parse_capabilities() gets called, which performs initialization > of some variables for DP link training (essential to get the DP going!). I think that's wrong. > The question that I am making to myself is whether I should move that elsewhere, > if so, what's the right place (making me able to remove the DRM_BRIDGE_OP_EDID > bridge flag when eDP + aux-bus), or if I should leave that and make sure that the > panel-edp's resume callback is called before .get_edid() from mtk-dp gets called. I think that's a proper change. Some of the callbacks in the DP driver are doing suspicious things, as if the driver came from an era without bridge drivers, and was not properly split up to fit the new bridge semantics. Same probably goes for the HPD detection. > That can get done in a separated series (or single patch?)... so that if we get > this one picked sooner than later, we can start upstreaming the panel nodes in > the Cherry devicetrees and only remove the regulator-always-on later. I guess that works? Except for battery life lol. It's really up to CK. And it's probably too late to upstream the panel nodes for the upcoming cycle. ChenYu
Il 04/04/23 12:47, AngeloGioacchino Del Regno ha scritto: Hello CK, Gentle ping for this series. Thanks, Angelo > Changes in v3: > - Added DPTX AUX block initialization before trying to communicate > to stop relying on the bootloader keeping it initialized before > booting Linux. > - Fixed commit description for patch [09/09] and removed commented > out code (that slipped from dev phase.. sorry!). > > This series adds "real" support for eDP in the mtk-dp DisplayPort driver. > > Explaining the "real": > Before this change, the DisplayPort driver did support eDP to some > extent, but it was treating it entirely like a regular DP interface > which is partially fine, after all, embedded DisplayPort *is* actually > DisplayPort, but there might be some differences to account for... and > this is for both small performance improvements and, more importantly, > for correct functionality in some systems. > > Functionality first: > > One of the common differences found in various boards implementing eDP > and machines using an eDP panel is that many times the HPD line is not > connected. This *must* be accounted for: at startup, this specific IP > will raise a HPD interrupt (which should maybe be ignored... as it does > not appear to be a "real" event...) that will make the eDP panel to be > detected and to actually work but, after a suspend-resume cycle, there > will be no HPD interrupt (as there's no HPD line in my case!) producing > a functionality issue - specifically, the DP Link Training fails because > the panel doesn't get powered up, then it stays black and won't work > until rebooting the machine (or removing and reinserting the module I > think, but I haven't tried that). > > Now for.. both: > eDP panels are *e*DP because they are *not* removable (in the sense that > you can't unplug the cable without disassembling the machine, in which > case, the machine shall be powered down..!): this (correct) assumption > makes us able to solve some issues and to also gain a little performance > during PM operations. > > What was done here is: > - Caching the EDID if the panel is eDP: we're always going to read the > same data everytime, so we can just cache that (as it's small enough) > shortening PM resume times for the eDP driver instance; > - Always return connector_status_connected if it's eDP: non-removable > means connector_status_disconnected can't happen during runtime... > this also saves us some time and even power, as we won't have to > perform yet another power cycle of the HW; > - Added aux-bus support! > This makes us able to rely on panel autodetection from the EDID, > avoiding to add more and more panel timings to panel-edp and, even > better, allowing to use one panel node in devicetrees for multiple > variants of the same machine since, at that point, it's not important > to "preventively know" what panel we have (eh, it's autodetected...!). > > This was tested on a MT8195 Cherry Tomato Chromebook (panel-edp on aux-bus) > > > P.S.: For your own testing commodity, here's a reference devicetree: > &edp_tx { > status = "okay"; > > pinctrl-names = "default"; > pinctrl-0 = <&edptx_pins_default>; > > ports { > #address-cells = <1>; > #size-cells = <0>; > > port@0 { > reg = <0>; > edp_in: endpoint { > remote-endpoint = <&dp_intf0_out>; > }; > }; > > port@1 { > reg = <1>; > edp_out: endpoint { > data-lanes = <0 1 2 3>; > remote-endpoint = <&panel_in>; > }; > }; > }; > > aux-bus { > panel: panel { > compatible = "edp-panel"; > power-supply = <&pp3300_disp_x>; > backlight = <&backlight_lcd0>; > port { > panel_in: endpoint { > remote-endpoint = <&edp_out>; > }; > }; > }; > }; > }; > > AngeloGioacchino Del Regno (9): > drm/mediatek: dp: Cache EDID for eDP panel > drm/mediatek: dp: Move AUX and panel poweron/off sequence to function > drm/mediatek: dp: Always return connected status for eDP in .detect() > drm/mediatek: dp: Always set cable_plugged_in at resume for eDP panel > drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() > drm/mediatek: dp: Enable event interrupt only when bridge attached > drm/mediatek: dp: Use devm variant of drm_bridge_add() > drm/mediatek: dp: Move AUX_P0 setting to > mtk_dp_initialize_aux_settings() > drm/mediatek: dp: Add support for embedded DisplayPort aux-bus > > drivers/gpu/drm/mediatek/mtk_dp.c | 186 +++++++++++++++++++----------- > 1 file changed, 116 insertions(+), 70 deletions(-) >