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 X-Spam-Level: X-Spam-Status: No, score=-6.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF300C433B4 for ; Tue, 11 May 2021 17:19:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9B080611F0 for ; Tue, 11 May 2021 17:19:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231462AbhEKRUX (ORCPT ); Tue, 11 May 2021 13:20:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59422 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230315AbhEKRUU (ORCPT ); Tue, 11 May 2021 13:20:20 -0400 Received: from mail-ot1-x32f.google.com (mail-ot1-x32f.google.com [IPv6:2607:f8b0:4864:20::32f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2396BC061574 for ; Tue, 11 May 2021 10:19:11 -0700 (PDT) Received: by mail-ot1-x32f.google.com with SMTP id u25-20020a0568302319b02902ac3d54c25eso18184007ote.1 for ; Tue, 11 May 2021 10:19:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:in-reply-to:references:from:user-agent:date:message-id :subject:to:cc; bh=go1LaTn+JyfnNbXmuN0CCBxYpt4cKSlPmbuS0BOCjb0=; b=J4uvkzvTwPOWZm01x9dBVdARTF4FC3M1Fw6NLUa6fADbc8UGlRb0heggAbMVyzPnw0 Jv2YMdvUjfg2VbwFHUh0HuQcDmhwyZZvNh2qFFVvKZSiLPl7xweJwXBUGHhRhHOjoY2u fHsKABo0FF/V0f5rzvMwCPePdU/bmXJEgfYuc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from :user-agent:date:message-id:subject:to:cc; bh=go1LaTn+JyfnNbXmuN0CCBxYpt4cKSlPmbuS0BOCjb0=; b=rmSI2Cn+s3h8hPr38nqI/4hbD7j5z8lQpPJe/90t9Hy4VwBcuihDH//4KB6EK3jrsG Afsg6Umv3ZbWZTGHY+XHSXwpu299YNI2bpSKmvNXYnAfnv0szrKMFmgbVcbOaUzeMC6k O2dK8gnfvLNOvhp/dQiktgNUMiq6r6q3/+c5JZCCWDbsmbbm56nk+z/63jipN4CSClPl YPAm0uSdz/ie8ICvQUvcmDWvI37hTjo235O70FFicHD9mvB/J56MHykfYVYEmfO1AsmS 0/8/F0e8Q/rSLEMUFmUGu1BiuSiuF1pW6bS4jEOrYZe8aa0oou72n93RZoB0eVv5dOdd ehvg== X-Gm-Message-State: AOAM532Gj4JQbZ8Sudi4B3lLywNvH0iVUKphXMlB2OYWDSojnLTdpz+t NDdmW1y9LuE35n2Lr66ryIC9j9e3QBC3GmDkZ72y6w== X-Google-Smtp-Source: ABdhPJy5ewWyxel0RSu/PYx4gRp0zStcp9SyVtIIlGhKV9pvJMqR7eBy8o8UoORh9nYbniwgUexDAdfjeKDbs958eWY= X-Received: by 2002:a05:6830:1556:: with SMTP id l22mr26940772otp.34.1620753550519; Tue, 11 May 2021 10:19:10 -0700 (PDT) Received: from 753933720722 named unknown by gmailapi.google.com with HTTPREST; Tue, 11 May 2021 10:19:09 -0700 MIME-Version: 1.0 In-Reply-To: References: <20210508074118.1621729-1-swboyd@chromium.org> From: Stephen Boyd User-Agent: alot/0.9.1 Date: Tue, 11 May 2021 10:19:09 -0700 Message-ID: Subject: Re: [PATCH] component: Move host device to end of device lists on binding To: Daniel Vetter , "Rafael J. Wysocki" Cc: Greg Kroah-Hartman , Linux Kernel Mailing List , Russell King , Rob Clark , dri-devel Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Quoting Daniel Vetter (2021-05-11 06:39:36) > On Tue, May 11, 2021 at 12:52 PM Rafael J. Wysocki wrote: > > > > On Mon, May 10, 2021 at 9:08 PM Stephen Boyd wrote: > > > > [cut] > > > > > > > > > > > > > > I will try it, but then I wonder about things like system wide > > > > > suspend/resume too. The drm encoder chain would need to reimplement the > > > > > logic for system wide suspend/resume so that any PM ops attached to the > > > > > msm device run in the correct order. Right now the bridge PM ops will > > > > > run, the i2c bus PM ops will run, and then the msm PM ops will run. > > > > > After this change, the msm PM ops will run, the bridge PM ops will run, > > > > > and then the i2c bus PM ops will run. It feels like that could be a > > > > > problem if we're suspending the DSI encoder while the bridge is still > > > > > active. > > > > > > > > Yup suspend/resume has the exact same problem as shutdown. > > > > > > I think suspend/resume has the exact opposite problem. At least I think > > > the correct order is to suspend the bridge, then the encoder, i.e. DSI, > > > like is happening today. It looks like drm_atomic_helper_shutdown() > > > operates from the top down when we want bottom up? I admit I have no > > > idea what is supposed to happen here. > > > > Why would the system-wide suspend ordering be different from the > > shutdown ordering? > > At least my point was that both shutdown and suspend/resume have the > same problem, and the righ fix is (I think at least) to add these > hooks to the component.c aggregate ops structure. Hence just adding > new callbacks for shutdown will be an incomplete solution. To add proper hooks to component.c we'll need to make the aggregate device into a 'struct device' and make a bus for them that essentially adds the aggregate device to the bus once all the components are registered. The bind/unbind can be ported to probe/remove, and then the aggregate driver can get PM ops that run before the component devices run their PM ops. Let me go try it out and see if I can make it minimally invasive so that the migration path is simple. > > I don't feel like changing the global device order is the right > approach, since essentially that's what component was meant to fix. > Except it's incomplete since it only provides a solution for > bind/unbind and not for shutdown or suspend/resume as other global > state changes. I think some drivers "fixed" this by putting stuff like > drm_atomic_helper_shutdown/suspend/resume into early/late hooks, to > make sure that everything is ready with that trick. But that doesn't > compose very well :-/ Yeah it looks like msm is using prepare/complete for this so that it can jump in early and suspend the display pipeline before the components suspend themselves. The shutdown path only has one callback so we can't play the same games. 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 X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9AA50C433ED for ; Tue, 11 May 2021 17:19:13 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 1F68A61352 for ; Tue, 11 May 2021 17:19:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1F68A61352 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5C42A6E7D5; Tue, 11 May 2021 17:19:12 +0000 (UTC) Received: from mail-ot1-x332.google.com (mail-ot1-x332.google.com [IPv6:2607:f8b0:4864:20::332]) by gabe.freedesktop.org (Postfix) with ESMTPS id 28F956E7D5 for ; Tue, 11 May 2021 17:19:11 +0000 (UTC) Received: by mail-ot1-x332.google.com with SMTP id t4-20020a05683014c4b02902ed26dd7a60so8008446otq.7 for ; Tue, 11 May 2021 10:19:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:in-reply-to:references:from:user-agent:date:message-id :subject:to:cc; bh=go1LaTn+JyfnNbXmuN0CCBxYpt4cKSlPmbuS0BOCjb0=; b=J4uvkzvTwPOWZm01x9dBVdARTF4FC3M1Fw6NLUa6fADbc8UGlRb0heggAbMVyzPnw0 Jv2YMdvUjfg2VbwFHUh0HuQcDmhwyZZvNh2qFFVvKZSiLPl7xweJwXBUGHhRhHOjoY2u fHsKABo0FF/V0f5rzvMwCPePdU/bmXJEgfYuc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from :user-agent:date:message-id:subject:to:cc; bh=go1LaTn+JyfnNbXmuN0CCBxYpt4cKSlPmbuS0BOCjb0=; b=FTnBi0t7t+ZEpPz5On2u347FR65qEybQxnnZapDmoCQKzRVypJkIlwpEVNTQS/uovH ilMLBaXsNBhdqVmu+iXxP+3X9KXbgYmn/icYb7sEWucnbH/CjO7wf+8UB24pGXJ0FT/E Hgzq5qfRgzAjeBg9qVb+RCvQR1IJt+8DLXv9dIFBfo76ENsNCUCx7y0x81pPsoMgsxGT YQqNpehOUrEpZ9G9q1cObyZkpqRkgP6qSJxHCyTwAEJ2Wl5bcULAF4/SugECkpbBKhsa F6QI0Qny8GRQd2aJCIBBtkgVvM+nNmIJxgi1ElyDvexFMMpS1dt8Qz5SvSP6yD/4RqtE 5Hog== X-Gm-Message-State: AOAM53047p94Fyva7FYX1pecmV27lImlD5u/oUtFlvXtU/xkSu6h641H say353qFFcN0MXqhBByIR52ZgSSq5MOLUE0p94CmLA== X-Google-Smtp-Source: ABdhPJy5ewWyxel0RSu/PYx4gRp0zStcp9SyVtIIlGhKV9pvJMqR7eBy8o8UoORh9nYbniwgUexDAdfjeKDbs958eWY= X-Received: by 2002:a05:6830:1556:: with SMTP id l22mr26940772otp.34.1620753550519; Tue, 11 May 2021 10:19:10 -0700 (PDT) Received: from 753933720722 named unknown by gmailapi.google.com with HTTPREST; Tue, 11 May 2021 10:19:09 -0700 MIME-Version: 1.0 In-Reply-To: References: <20210508074118.1621729-1-swboyd@chromium.org> From: Stephen Boyd User-Agent: alot/0.9.1 Date: Tue, 11 May 2021 10:19:09 -0700 Message-ID: Subject: Re: [PATCH] component: Move host device to end of device lists on binding To: Daniel Vetter , "Rafael J. Wysocki" 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: Greg Kroah-Hartman , Linux Kernel Mailing List , dri-devel , Russell King Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Quoting Daniel Vetter (2021-05-11 06:39:36) > On Tue, May 11, 2021 at 12:52 PM Rafael J. Wysocki wrote: > > > > On Mon, May 10, 2021 at 9:08 PM Stephen Boyd wrote: > > > > [cut] > > > > > > > > > > > > > > I will try it, but then I wonder about things like system wide > > > > > suspend/resume too. The drm encoder chain would need to reimplement the > > > > > logic for system wide suspend/resume so that any PM ops attached to the > > > > > msm device run in the correct order. Right now the bridge PM ops will > > > > > run, the i2c bus PM ops will run, and then the msm PM ops will run. > > > > > After this change, the msm PM ops will run, the bridge PM ops will run, > > > > > and then the i2c bus PM ops will run. It feels like that could be a > > > > > problem if we're suspending the DSI encoder while the bridge is still > > > > > active. > > > > > > > > Yup suspend/resume has the exact same problem as shutdown. > > > > > > I think suspend/resume has the exact opposite problem. At least I think > > > the correct order is to suspend the bridge, then the encoder, i.e. DSI, > > > like is happening today. It looks like drm_atomic_helper_shutdown() > > > operates from the top down when we want bottom up? I admit I have no > > > idea what is supposed to happen here. > > > > Why would the system-wide suspend ordering be different from the > > shutdown ordering? > > At least my point was that both shutdown and suspend/resume have the > same problem, and the righ fix is (I think at least) to add these > hooks to the component.c aggregate ops structure. Hence just adding > new callbacks for shutdown will be an incomplete solution. To add proper hooks to component.c we'll need to make the aggregate device into a 'struct device' and make a bus for them that essentially adds the aggregate device to the bus once all the components are registered. The bind/unbind can be ported to probe/remove, and then the aggregate driver can get PM ops that run before the component devices run their PM ops. Let me go try it out and see if I can make it minimally invasive so that the migration path is simple. > > I don't feel like changing the global device order is the right > approach, since essentially that's what component was meant to fix. > Except it's incomplete since it only provides a solution for > bind/unbind and not for shutdown or suspend/resume as other global > state changes. I think some drivers "fixed" this by putting stuff like > drm_atomic_helper_shutdown/suspend/resume into early/late hooks, to > make sure that everything is ready with that trick. But that doesn't > compose very well :-/ Yeah it looks like msm is using prepare/complete for this so that it can jump in early and suspend the display pipeline before the components suspend themselves. The shutdown path only has one callback so we can't play the same games.