From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5877CC433F5 for ; Tue, 5 Apr 2022 18:12:00 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3A10D10EE33; Tue, 5 Apr 2022 18:11:59 +0000 (UTC) Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7E34F10EE31 for ; Tue, 5 Apr 2022 18:11:57 +0000 (UTC) Received: by mail-ej1-x62a.google.com with SMTP id k23so24950988ejd.3 for ; Tue, 05 Apr 2022 11:11:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=660LH2+GxfU/Tt425A6dqO9O0qLu2gGwDombZrfKpU8=; b=gwNC/cNNASSPrm/edmwXPk0X7RaQdKKSivzCUoyBbji7ak6C8kchHEP5egX8PkIP4C sazrZ+UZmtaVXMpC4qrc5t0H+eIPolaDcFXOFq2kJoDEq0rKnwf93q0H04FDyVTlple4 HndygiAgVBA2rSkB38vxENQIHCisJHKkZm1Uk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=660LH2+GxfU/Tt425A6dqO9O0qLu2gGwDombZrfKpU8=; b=Y16mEYfZrnLXkevnTBYbMiVlnZYsOxN3r9LHatzFhdWOdffTxUi5EeiAbTrmkmSEBo W2GTwmnT7d3lKTNTkaXMDY0cl/KqqGNPFlItEUAB5bI5KQFZUMBRYcTjkGxCiDv0IaPq ue4xnMGRqm/+ZUBOGMAEzX1C4kCKuB+L175mxcnPuZXo2isaYgjPFwn2sl3L9lTPx858 C8Tl8q/2/tJC7gwNuU194gJdVRyFP/Xt7687qIeL5wBaUawQTz+k5Ymxaa4aZIT06ydU JkQU/uiauoh/VTW9HuSdfx2hkydJ9lZ1ZXKmOQZQY0yaQfeuYaZaZ2NvsJHWRM2pAo44 azyg== X-Gm-Message-State: AOAM532i9bSy8C5rzY30cxjQOm7jvupwKUKz33O/NofX/Dr+kNYDnhHr nqAwnUcFo/KOWTIqyCTqFrXZR204QpPAVhCm X-Google-Smtp-Source: ABdhPJyhWX6lJFTRmbfTSW097fH62g8GrypEdNvi071d7KKF629ZroJ/PTw3Lc7YTDe/YnSCML3UUw== X-Received: by 2002:a17:906:1615:b0:6bb:150f:adf8 with SMTP id m21-20020a170906161500b006bb150fadf8mr4719160ejd.272.1649182315546; Tue, 05 Apr 2022 11:11:55 -0700 (PDT) Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com. [209.85.221.49]) by smtp.gmail.com with ESMTPSA id et21-20020a170907295500b006e7f1abe2ccsm2612150ejc.75.2022.04.05.11.11.53 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 05 Apr 2022 11:11:54 -0700 (PDT) Received: by mail-wr1-f49.google.com with SMTP id w4so20530584wrg.12 for ; Tue, 05 Apr 2022 11:11:53 -0700 (PDT) X-Received: by 2002:adf:9123:0:b0:205:f439:cbdf with SMTP id j32-20020adf9123000000b00205f439cbdfmr3560336wrj.513.1649182313067; Tue, 05 Apr 2022 11:11:53 -0700 (PDT) MIME-Version: 1.0 References: <1648656179-10347-1-git-send-email-quic_sbillaka@quicinc.com> <1648656179-10347-2-git-send-email-quic_sbillaka@quicinc.com> <392b933f-760c-3c81-1040-c514045df3da@linaro.org> <3e5fa57f-d636-879a-b98f-77323d07c156@linaro.org> In-Reply-To: <3e5fa57f-d636-879a-b98f-77323d07c156@linaro.org> From: Doug Anderson Date: Tue, 5 Apr 2022 11:11:40 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus To: Dmitry Baryshkov Content-Type: text/plain; charset="UTF-8" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: quic_kalyant , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Sankeerth Billakanti , "Abhinav Kumar \(QUIC\)" , quic_vproddut , David Airlie , linux-arm-msm , Bjorn Andersson , LKML , dri-devel , Stephen Boyd , Sean Paul , Sean Paul , "Aravind Venkateswaran \(QUIC\)" , "Kuogee Hsieh \(QUIC\)" , freedreno Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi, On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov wrote: > > On 05/04/2022 20:02, Doug Anderson wrote: > > Hi, > > > > On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov > > wrote: > >>> 3. For DP and eDP HPD means something a little different. Essentially > >>> there are two concepts: a) is a display physically connected and b) is > >>> the display powered up and ready. For DP, the two are really tied > >>> together. From the kernel's point of view you never "power down" a DP > >>> display and you can't detect that it's physically connected until it's > >>> ready. Said another way, on you tie "is a display there" to the HPD > >>> line and the moment a display is there it's ready for you to do AUX > >>> transfers. For eDP, in the lowest power state of a display it _won't_ > >>> assert its "HPD" signal. However, it's still physically present. For > >>> eDP you simply have to _assume_ it's present without any actual proof > >>> since you can't get proof until you power it up. Thus for eDP, you > >>> report that the display is there as soon as we're asked. We can't > >>> _talk_ to the display yet, though. So in get_modes() we need to be > >>> able to power the display on enough to talk over the AUX channel to > >>> it. As part of this, we wait for the signal named "HPD" which really > >>> means "panel finished powering on" in this context. > >>> > >>> NOTE: for aux transfer, we don't have the _display_ pipe and clocks > >>> running. We only have enough stuff running to do the AUX transfer. > >>> We're not clocking out pixels. We haven't fully powered on the > >>> display. The AUX transfer is designed to be something that can be done > >>> early _before_ you turn on the display. > >>> > >>> > >>> OK, so basically that was a longwinded way of saying: yes, we could > >>> avoid the AUX transfer in probe, but we can't wait all the way to > >>> enable. We have to be able to transfer in get_modes(). If you think > >>> that's helpful I think it'd be a pretty easy patch to write even if it > >>> would look a tad bit awkward IMO. Let me know if you want me to post > >>> it up. > >> > >> I think it would be a good idea. At least it will allow us to judge, > >> which is the more correct way. > > > > I'm still happy to prototype this, but the more I think about it the > > more it feels like a workaround for the Qualcomm driver. The eDP panel > > driver is actually given a pointer to the AUX bus at probe time. It's > > really weird to say that we can't do a transfer on it yet... As you > > said, this is a little sideband bus. It should be able to be used > > without all the full blown infra of the rest of the driver. > > Yes, I have that feeling too. However I also have a feeling that just > powering up the PHY before the bus probe is ... a hack. There are no > obvious stopgaps for the driver not to power it down later. This is why I think we need to move to Runtime PM to manage this. Basically: 1. When an AUX transfer happens, you grab a PM runtime reference that _that_ powers up the PHY. 2. At the end of the AUX transfer function, you do a "put_autosuspend". Then it becomes not a hack, right? > A cleaner design might be to split all hotplug event handling from the > dp_display, provide a lightweight state machine for the eDP and select > which state machine to use depending on the hardware connector type. The > dp_display_bind/unbind would probably also be duplicated and receive > correct code flows for calling dp_parser_get_next_bridge, etc. > Basically that means that depending on the device data we'd use either > dp_display_comp_ops or (new) edp_comp_ops. > > WDYT? I don't think I know the structure of the MSM DP code to make a definitive answer here. I think I'd have to see a patch. However I'd agree in general terms that we need some different flows for the two. ;-) We definitely want to limit the differences but some of them will be unavoidable... -Doug From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 580A9C3527B for ; Tue, 5 Apr 2022 23:34:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1575517AbiDEXIF (ORCPT ); Tue, 5 Apr 2022 19:08:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52054 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1573187AbiDESN6 (ORCPT ); Tue, 5 Apr 2022 14:13:58 -0400 Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 151F986E3D for ; Tue, 5 Apr 2022 11:11:59 -0700 (PDT) Received: by mail-ed1-x52d.google.com with SMTP id w18so9628254edi.13 for ; Tue, 05 Apr 2022 11:11:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=660LH2+GxfU/Tt425A6dqO9O0qLu2gGwDombZrfKpU8=; b=gwNC/cNNASSPrm/edmwXPk0X7RaQdKKSivzCUoyBbji7ak6C8kchHEP5egX8PkIP4C sazrZ+UZmtaVXMpC4qrc5t0H+eIPolaDcFXOFq2kJoDEq0rKnwf93q0H04FDyVTlple4 HndygiAgVBA2rSkB38vxENQIHCisJHKkZm1Uk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=660LH2+GxfU/Tt425A6dqO9O0qLu2gGwDombZrfKpU8=; b=2VWIRBg4Ap8lMagZ0fqJVUfVv7ehWzOhWfFtnb45DykkzIszqVwez5jfq7fqx3c5p6 RfFZP90UVCs0aWp3qkjcHAEeKXp4AXvhxi/hqqEdhYIsHC635FxTKbeWlewoedW97me2 Cv2yR2OeDPdjTZnKhfwBL643XapMGtZpobGp5/WyYiBzMIPECF9+k0lDj4wgZ9Iq/gUw LMc+mvZlKYLtvKMutWNeS6Gqbc0DXMSbkejMgfwFBSHQxp+JeP93vm4qXDxBZAhORVHH lRX3u2WzDXpqb7IHCokAHOObpOqjAb2MOJ8CT56Lek7QEkeD3Fa+UMlxRB+E+h8a44P4 ajxg== X-Gm-Message-State: AOAM531k0cCi35eKG5nuTy3GoFKBoDPIHHqqUkIdFGvZVJpf68APEi5p NK6aOZ1UAoyu84kZYVLMEPfFlZsqcUb7jMwt X-Google-Smtp-Source: ABdhPJxMHvqJiaaIHDacozQgHWEmN0buTDlQ6dwk4aZj8HbwPGx68EHLUg+9taMt0M54Os/Z406Jlw== X-Received: by 2002:aa7:d553:0:b0:416:4dfc:126d with SMTP id u19-20020aa7d553000000b004164dfc126dmr4897617edr.213.1649182317233; Tue, 05 Apr 2022 11:11:57 -0700 (PDT) Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com. [209.85.221.50]) by smtp.gmail.com with ESMTPSA id hp11-20020a1709073e0b00b006dfd53a0e39sm5725645ejc.135.2022.04.05.11.11.53 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 05 Apr 2022 11:11:54 -0700 (PDT) Received: by mail-wr1-f50.google.com with SMTP id c7so20697452wrd.0 for ; Tue, 05 Apr 2022 11:11:53 -0700 (PDT) X-Received: by 2002:adf:9123:0:b0:205:f439:cbdf with SMTP id j32-20020adf9123000000b00205f439cbdfmr3560336wrj.513.1649182313067; Tue, 05 Apr 2022 11:11:53 -0700 (PDT) MIME-Version: 1.0 References: <1648656179-10347-1-git-send-email-quic_sbillaka@quicinc.com> <1648656179-10347-2-git-send-email-quic_sbillaka@quicinc.com> <392b933f-760c-3c81-1040-c514045df3da@linaro.org> <3e5fa57f-d636-879a-b98f-77323d07c156@linaro.org> In-Reply-To: <3e5fa57f-d636-879a-b98f-77323d07c156@linaro.org> From: Doug Anderson Date: Tue, 5 Apr 2022 11:11:40 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus To: Dmitry Baryshkov Cc: Sankeerth Billakanti , dri-devel , linux-arm-msm , freedreno , LKML , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Rob Clark , Sean Paul , Stephen Boyd , quic_kalyant , "Abhinav Kumar (QUIC)" , "Kuogee Hsieh (QUIC)" , Bjorn Andersson , Sean Paul , David Airlie , Daniel Vetter , quic_vproddut , "Aravind Venkateswaran (QUIC)" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Hi, On Tue, Apr 5, 2022 at 10:36 AM Dmitry Baryshkov wrote: > > On 05/04/2022 20:02, Doug Anderson wrote: > > Hi, > > > > On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov > > wrote: > >>> 3. For DP and eDP HPD means something a little different. Essentially > >>> there are two concepts: a) is a display physically connected and b) is > >>> the display powered up and ready. For DP, the two are really tied > >>> together. From the kernel's point of view you never "power down" a DP > >>> display and you can't detect that it's physically connected until it's > >>> ready. Said another way, on you tie "is a display there" to the HPD > >>> line and the moment a display is there it's ready for you to do AUX > >>> transfers. For eDP, in the lowest power state of a display it _won't_ > >>> assert its "HPD" signal. However, it's still physically present. For > >>> eDP you simply have to _assume_ it's present without any actual proof > >>> since you can't get proof until you power it up. Thus for eDP, you > >>> report that the display is there as soon as we're asked. We can't > >>> _talk_ to the display yet, though. So in get_modes() we need to be > >>> able to power the display on enough to talk over the AUX channel to > >>> it. As part of this, we wait for the signal named "HPD" which really > >>> means "panel finished powering on" in this context. > >>> > >>> NOTE: for aux transfer, we don't have the _display_ pipe and clocks > >>> running. We only have enough stuff running to do the AUX transfer. > >>> We're not clocking out pixels. We haven't fully powered on the > >>> display. The AUX transfer is designed to be something that can be done > >>> early _before_ you turn on the display. > >>> > >>> > >>> OK, so basically that was a longwinded way of saying: yes, we could > >>> avoid the AUX transfer in probe, but we can't wait all the way to > >>> enable. We have to be able to transfer in get_modes(). If you think > >>> that's helpful I think it'd be a pretty easy patch to write even if it > >>> would look a tad bit awkward IMO. Let me know if you want me to post > >>> it up. > >> > >> I think it would be a good idea. At least it will allow us to judge, > >> which is the more correct way. > > > > I'm still happy to prototype this, but the more I think about it the > > more it feels like a workaround for the Qualcomm driver. The eDP panel > > driver is actually given a pointer to the AUX bus at probe time. It's > > really weird to say that we can't do a transfer on it yet... As you > > said, this is a little sideband bus. It should be able to be used > > without all the full blown infra of the rest of the driver. > > Yes, I have that feeling too. However I also have a feeling that just > powering up the PHY before the bus probe is ... a hack. There are no > obvious stopgaps for the driver not to power it down later. This is why I think we need to move to Runtime PM to manage this. Basically: 1. When an AUX transfer happens, you grab a PM runtime reference that _that_ powers up the PHY. 2. At the end of the AUX transfer function, you do a "put_autosuspend". Then it becomes not a hack, right? > A cleaner design might be to split all hotplug event handling from the > dp_display, provide a lightweight state machine for the eDP and select > which state machine to use depending on the hardware connector type. The > dp_display_bind/unbind would probably also be duplicated and receive > correct code flows for calling dp_parser_get_next_bridge, etc. > Basically that means that depending on the device data we'd use either > dp_display_comp_ops or (new) edp_comp_ops. > > WDYT? I don't think I know the structure of the MSM DP code to make a definitive answer here. I think I'd have to see a patch. However I'd agree in general terms that we need some different flows for the two. ;-) We definitely want to limit the differences but some of them will be unavoidable... -Doug