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 E2843C43217 for ; Fri, 8 Apr 2022 14:18:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235431AbiDHOUL (ORCPT ); Fri, 8 Apr 2022 10:20:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56712 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236795AbiDHOUC (ORCPT ); Fri, 8 Apr 2022 10:20:02 -0400 Received: from mail-ed1-x533.google.com (mail-ed1-x533.google.com [IPv6:2a00:1450:4864:20::533]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C102D1B255C for ; Fri, 8 Apr 2022 07:17:56 -0700 (PDT) Received: by mail-ed1-x533.google.com with SMTP id b15so10261533edn.4 for ; Fri, 08 Apr 2022 07:17:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=z2JJx5IiDtTwR6XIUL714OcPUvnCKRiD1BViuooLypE=; b=CGpI14i0IwPVUyQqUyjByEDEtvkduDKUP7QuMKRoP5X6taKf8CuagqxSKl0sUVwrJ6 XlNiwJVxLEaaLeESNo1108Ey5YpNqQwtI1MMVoK33xpMfv+ntnywV4+AdxcBtxaqUtiB v42lwnS+0NBcuvOdUFBFjSBaaG8u4SGkaHFNJzd5yma1QbRjddoJECDuLr1ByP2jTcRO WUjiD6sVRM3XnHllzbQq21jnwdhNxTkp3IOe5AU8DEF8UtwpVbRkA2j5PR5A1E6FIetC ts/n6gziaQp7LcOyY+UuRl8qpSkmwjXuA5N4JjHJxcnPbeTQiwLbCN3+1JsTSwqTidt9 P5jA== 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=z2JJx5IiDtTwR6XIUL714OcPUvnCKRiD1BViuooLypE=; b=rWIekbXczLsMARgbxo6dnxYDo0aknAJSnqQQUx9n5FE+GHmc5HltxOcA5y3hrYvi+D 03oFIfvTaftr3MCjx9Yr5k4CnObzW9uJ5wfeFYCMSldEmnVEufxyrfYLRkLYJ88ROjyq +q6bHDGmYYbsFiUHQaYRuk5OFd8SjjLJmSQZIGJBtsSf2FKh+TAodFLVcp68jG9c99FL aiQ/ZRPcY7lqHyDVSZdb7cYMO6J9aqgpygcc65sGd6peDXgajSF//vph1YzyL5r4PbJQ AbsBdYoeFspYq67a+4YUHi1WprH69pVRPHbh23ZvYMTj+XLFRKviBO7inc40zqAeDzzB 3ldw== X-Gm-Message-State: AOAM530uOV/DCWhkr9DaKbtl7weaKGPXI5aRZw/Ht5QX6zE1UjoP1Lte 9QFqF7XloL5LZRn5FHNb8Lz7y9XE9GiIovdRHwMM1lFnUhk= X-Google-Smtp-Source: ABdhPJwthrucugVECJPFqMMkPdoQSMiDpbnsnpjUekEkrPs3/zxUp6NmoUpnfog0HvMvVa9IrZ6w7Hyd3rCSGbzbmeA= X-Received: by 2002:a05:6402:1804:b0:41d:5883:2f70 with SMTP id g4-20020a056402180400b0041d58832f70mr1242754edy.150.1649427475320; Fri, 08 Apr 2022 07:17:55 -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: From: Dmitry Baryshkov Date: Fri, 8 Apr 2022 17:17:43 +0300 Message-ID: Subject: Re: [PATCH v6 1/8] drm/msm/dp: Add eDP support via aux_bus To: Doug Anderson Cc: Abhinav Kumar , "Sankeerth Billakanti (QUIC)" , quic_kalyant , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , quic_vproddut , David Airlie , linux-arm-msm , "Kuogee Hsieh (QUIC)" , freedreno , dri-devel , "bjorn.andersson@linaro.org" , Sean Paul , "Aravind Venkateswaran (QUIC)" , Stephen Boyd , Sean Paul , LKML Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 8 Apr 2022 at 16:56, Doug Anderson wrote: > > Hi, > > On Fri, Apr 8, 2022 at 5:13 AM Dmitry Baryshkov > wrote: > > > > On Fri, 8 Apr 2022 at 03:28, Doug Anderson wrote: > > > > > > Hi, > > > > > > On Thu, Apr 7, 2022 at 4:36 PM Dmitry Baryshkov > > > wrote: > > > > > > > > The ps8640 driver looks 'working by coincidence'. It calls > > > > dp_aux_populate, then immediately after the function returns it checks > > > > for the panel. If panel-edp is built as a module, the probe might fail > > > > easily. > > > > The anx7625 driver has the same kind of issue. The DP AUX bus is > > > > populated from the probe() and after some additional work the panel is > > > > being checked. > > > > This design is fragile and from my quick glance it can break (or be > > > > broken) too easy. It reminds me of our drm msm 'probe' loops > > > > preventing the device to boot completely if the dsi bridge/panel could > > > > not be probed in time. > > > > > > I did spend some time thinking about this, at least for ps8640. I > > > believe that as long as the panel's probe isn't asynchronous. > > > > By panel probe (as a probe of any device) is potentially asynchronous. > > As in your example, the PWM might not be present, the regulator probe > > might have been delayed, the panel-edp might be built as a module, > > which is not present for some reason. > > Well, in those cases it's not exactly asynchronous, or at least not in > the way I was thinking about. Either it will work now, or we should > try again later when more drivers have probed. The case I was worried > about was: > > 1. We call devm_of_dp_aux_populate_ep_devices() > > 2. devm_of_dp_aux_populate_ep_devices() kicks off a probe to the panel > in the background > > 3. devm_of_dp_aux_populate_ep_devices() returns to us without waiting > for the panel's probe to finish. > > I think that's possible if the panel driver sets PROBE_PREFER_ASYNCHRONOUS. That would be a separate story, yes. However the general case is still valid: one can not guarantee that the panel device is available immediately after aux bus population. So ps8640 works at this moment and in the particular configuration. > > I was less worried about the resources missing since I thought that > would be handled by the normal probe deferral case. IIRC the "infinite > loop" that we used to end up with MSM's probe was because something > about the way the MSM DRM driver worked would cause the deferral > mechanisms to retry instantly each time we deferred. I don't remember > exactly what triggered it, but I don't _think_ that's true for ps8640? I'm not sure either. If you have a system with that bridge, it can be easily tried by returning -EPROBE_DEFER from the panel driver's probe(). For the msm driver it was the following sequence of events: - mdss probes - mdss creates subdevices including dsi host - subdevices are probed - mdss drivers tries to perform component binding - dsi host determines that the next item is not available - it returns -EPROBE_DEFER to the component bind - mdss reverts registration of subdevices, returning probe defer. However as there were devices added to the device list, the deferral list was retried immediately. Thus we faced the probe loop. I think this looks close to the ps8640, but I wouldn't bet on that. > > > Basically if the panel isn't ready then ps8640 should return and we'll > > > retry later. I do remember the probe loops that we used to have with > > > msm and I don't _think_ this would trigger it. > > > > I don't have proof here. But as I wrote, this thing might break at some point. > > > > > That being said, if we need to separate out the AUX bus into a > > > sub-device like we did in sn65dsi86 we certainly could. > > > > Ideally we should separate the "bridge" subdevice, like it was done in > > sn65dsi86. > > So that the aux host probes, registers the EP device, then the bridge > > device can not be probed (and thus the drm_bridge is not created) > > until the next bridge becomes available. > > You're definitely right, that's best. I was hesitant to force the > issue during review of the ps8640 because it adds a bunch of > complexity and didn't _seem_ to be needed. Maybe it makes sense to > just code it up, though... As I wrote, the test is easy. If the system boots fine, there is no need to fix that immediately. However I think in general we should stop depending on the panel being available right after populating the aux bus. -- With best wishes Dmitry