On Thu, Jul 13, 2023 at 5:57 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote: > > Il 13/07/23 11:54, Chen-Yu Tsai ha scritto: > > On Thu, Jul 13, 2023 at 5:01 PM AngeloGioacchino Del Regno > > <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote: > >> > >> Changes in v5: > >> - Added .wait_hpd_asserted() callback for aux-bus > >> - Avoid enabling and registering HPD interrupt + handlers for > >> eDP case only (keeps HPD interrupts enabled for full DP case) > >> - Support not always-on eDP panels (boot with regulator off, > >> suspend with regulator off) for power saving in PM suspend. > > > > This still doesn't work when the DRM driver is builtin, and the panel > > driver is a module. This is still with regulator-always-on. > > > > From what I can tell from the kernel logs, the DRM driver is not waiting > > for eDP panel to probe (which sort of makes sense?), and simply uses > > the default EDID. When the panel does probe, nothing triggers the DRM > > driver to update its connector. > > > > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:32:eDP-1] > > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:32:eDP-1] > > status updated from unknown to connected > > [drm:drm_mode_debug_printmodeline] Modeline "640x480": 60 25175 640 > > 656 752 800 480 490 492 525 0x40 0xa > > [drm:drm_mode_prune_invalid] Not using 640x480 mode: CLOCK_HIGH > > [drm:drm_mode_debug_printmodeline] Modeline "800x600": 56 36000 800 > > 824 896 1024 600 601 603 625 0x40 0x5 > > [drm:drm_mode_prune_invalid] Not using 800x600 mode: CLOCK_HIGH > > [drm:drm_mode_debug_printmodeline] Modeline "800x600": 60 40000 800 > > 840 968 1056 600 601 605 628 0x40 0x5 > > [drm:drm_mode_prune_invalid] Not using 800x600 mode: CLOCK_HIGH > > [drm:drm_mode_debug_printmodeline] Modeline "848x480": 60 33750 848 > > 864 976 1088 480 486 494 517 0x40 0x5 > > [drm:drm_mode_prune_invalid] Not using 848x480 mode: CLOCK_HIGH > > [drm:drm_mode_debug_printmodeline] Modeline "1024x768": 60 65000 1024 > > 1048 1184 1344 768 771 777 806 0x40 0xa > > [drm:drm_mode_prune_invalid] Not using 1024x768 mode: CLOCK_HIGH > > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1] > > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1] > > status updated from unknown to disconnected > > [drm:drm_helper_probe_single_connector_modes] [CONNECTOR:34:DP-1] disconnected > > [drm:drm_client_modeset_probe] No connectors reported connected with modes > > [drm:drm_client_modeset_probe] connector 32 enabled? yes > > [drm:drm_client_modeset_probe] connector 34 enabled? no > > [drm:drm_client_firmware_config.constprop.0.isra.0] Not using firmware > > configuration > > [drm:drm_client_modeset_probe] looking for cmdline mode on connector 32 > > [drm:drm_client_modeset_probe] looking for preferred mode on connector 32 0 > > [drm:drm_client_modeset_probe] found mode none > > [drm:drm_client_modeset_probe] picking CRTCs for 4096x4096 config > > mediatek-drm mediatek-drm.12.auto: > > [drm:__drm_fb_helper_initial_config_and_unlock] test CRTC 0 primary > > plane > > mediatek-drm mediatek-drm.12.auto: [drm] Cannot find any crtc or sizes > > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp: > > 0x00000 AUX -> (ret= 1) 14 > > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp: > > 0x00000 AUX -> (ret= 15) 14 0a 84 41 00 00 01 80 02 00 00 00 0f 09 80 > > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp: > > 0x00000 AUX -> (ret= 1) 14 > > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp: > > 0x02200 AUX -> (ret= 15) 14 0a 84 41 00 00 01 80 02 00 00 00 0f 01 80 > > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_read_dpcd_caps] > > aux_mtk_dp: Base DPCD: 14 0a 84 41 00 00 01 80 02 00 00 00 0f 09 80 > > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_read_dpcd_caps] > > aux_mtk_dp: DPCD: 14 0a 84 41 00 00 01 80 02 00 00 00 0f 01 80 > > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_probe] aux_mtk_dp: > > 0x00000 AUX -> (ret= 1) 14 > > mediatek-drm mediatek-drm.12.auto: [drm:drm_dp_dpcd_read] aux_mtk_dp: > > 0x00021 AUX -> (ret= 1) 00 > > panel-simple-dp-aux aux-1c500000.edp-tx: Detected BOE NE135FBM-N41 v8.1 (0x095f) > > > > If the panel is also built-in, then the eDP panel probe happens between > > the drm driver adding components and binding to them, and everything seems > > to work OK. > > > > Argh. I actually forgot to test that case. Sorry about that. > > Anyway, you don't need regulator-always-on anymore, nor regulator-boot-on... Confirmed this case works now. > ...I'll recheck with panel-edp as module and fix. With the module case, I also get an devlink error: mediatek-drm-dp 1c500000.edp-tx: Failed to create device link (0x180) with regulator-pp3300-disp-x which doesn't happen with the builtin case. Not sure if it gets a retry or anything later. ChenYu > Cheers, > Angelo > > > > > ChenYu > > > >> Changes in v4: > >> - Set data lanes to idle to prevent stalls if bootloader didn't > >> properly close the eDP port > >> - Now using the .done_probing() callback for AUX bus to prevent > >> probe deferral loops in case the panel-edp driver is a module > >> as previously seen with another bridge driver (ANX7625) on > >> some other SoCs (MT8192 and others) > >> - Rebased over next-20230706 > >> - Dropped Chen-Yu's T-b tag on last patch as some logic changed > >> (before, I wasn't using the .done_probing() callback). > >> > >> 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: > >> > >> pp3300_disp_x: regulator-pp3300-disp-x { > >> compatible = "regulator-fixed"; > >> regulator-name = "pp3300_disp_x"; > >> regulator-min-microvolt = <3300000>; > >> regulator-max-microvolt = <3300000>; > >> enable-active-high; > >> gpio = <&pio 55 GPIO_ACTIVE_HIGH>; > >> pinctrl-names = "default"; > >> pinctrl-0 = <&panel_fixed_pins>; > >> }; > >> > >> &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 (10): > >> drm/mediatek: dp: Move AUX and panel poweron/off sequence to function > >> drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer() > >> 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: Enable event interrupt only when bridge attached > >> drm/mediatek: dp: Avoid mutex locks if audio is not supported/enabled > >> drm/mediatek: dp: Move PHY registration to new function > >> drm/mediatek: dp: Add support for embedded DisplayPort aux-bus > >> drm/mediatek: dp: Add .wait_hpd_asserted() for AUX bus > >> drm/mediatek: dp: Don't register HPD interrupt handler for eDP case > >> > >> drivers/gpu/drm/mediatek/Kconfig | 1 + > >> drivers/gpu/drm/mediatek/mtk_dp.c | 335 ++++++++++++++++++++---------- > >> 2 files changed, 224 insertions(+), 112 deletions(-) > >> > >> -- > >> 2.40.1 > >> > > >