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;
Message-ID: <6d870923-ce25-08f6-c3aa-453a4737953b@collabora.com>
Date:   Tue, 30 May 2023 08:52:06 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Subject: Re: [PATCH v3 0/9] MediaTek DisplayPort: support eDP and aux-bus
Content-Language: en-US
To:     chunkuang.hu@kernel.org
Cc:     p.zabel@pengutronix.de, airlied@gmail.com, daniel@ffwll.ch,
        matthias.bgg@gmail.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
References: <20230404104800.301150-1-angelogioacchino.delregno@collabora.com>
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
In-Reply-To: <20230404104800.301150-1-angelogioacchino.delregno@collabora.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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(-)
>