Received: by 2002:a05:6a10:1a4d:0:0:0:0 with SMTP id nk13csp898272pxb; Tue, 1 Feb 2022 12:43:29 -0800 (PST) X-Google-Smtp-Source: ABdhPJzAHWQCM99LYbXE2T+mmGD3P9Pm0lVfMq/XzjV9RXOAunA9EdqbW4hUkV/YxfQ4GhZUOijW X-Received: by 2002:a63:8ac9:: with SMTP id y192mr22131021pgd.598.1643748208908; Tue, 01 Feb 2022 12:43:28 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1643748208; cv=none; d=google.com; s=arc-20160816; b=XwxDgDJiQNlfRY1cbtNahaAVx/lQYsDQ5/U0iWMod8ap0AVyeFBcJEHE+gkfUhIkZA mDPwK4Tu9PlbsZHEYxEJOa+QzURAm31lmZDktzg7gIzhjJjbZnRGnEvQaMwdP47Tm6h0 XyliJT8XpyQanQPp0Q88Jp2iMbxDOtUh5U+2XhBwyPrQ3j0nxamrYMugdGzlMgkUlvVz ag1x/gkjt2WOiSGfuWuFR1XNx7egLbx7ww/BA7e9hbIltdtn/ki3Lf3vE2I0Fu39SsDo fXIhL35kyQ6GPUErGVMTGT/3sefFlsyStus0zS3WdODQgCv6OSomGK1xR+yl27SSwIfu Bh/w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=bfUfsNLn8XfkSiYQS9JUSBWbvxQHvYzEpvse63WRdhs=; b=JW/RsigW6vwXB2h04v3qPTehUawnhsy2n1n2LT5wfQaA+WhU6kp9CgKdCVxJsKG49k le+Z8qoDFG4Wu6Vpd5urzl7u/O6c1h5rrAwgr07ulwz/zrsTGdIAlWPk4H4IWxqJSSI7 Lp8kjnicuGk+4zGJCBAlYFfPYx9iCZ/ElR99Vnu5ubSiFINcDEtYYuwEYdqOKIPguu4O YA1M0lQ+tldokevFwXm5AFT+gH8Ij1Al8d0N09wGC/RaVpWJaXN+7dUaDSfnUih+S4mH xxRgK+l36J/57Vki2eUsgnqnNQ3G0LzYn+4pHciqJx4lFmg1JD457tUbFcSusbck1zTv zegg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=PBaZx92j; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id lx9si3961589pjb.183.2022.02.01.12.43.17; Tue, 01 Feb 2022 12:43:28 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=PBaZx92j; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1379719AbiAaPd0 (ORCPT + 99 others); Mon, 31 Jan 2022 10:33:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51384 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1379665AbiAaPdP (ORCPT ); Mon, 31 Jan 2022 10:33:15 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 62179C061714; Mon, 31 Jan 2022 07:33:15 -0800 (PST) Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 03CE21447; Mon, 31 Jan 2022 16:33:12 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1643643193; bh=eMAheiDAIWDxrcMKCOAT+TMinkKp+mNuDD2YkGykmEA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PBaZx92jmIQ7yqnZiQLYizdQiTlGCn6AdWkP7elcXV9pe4j8FFi5kNz8+n1pBi7vU XLeOlunA+ixdDCKWBcEy7jaGXPvuRXBnFtOcdBOx2zkkgDsKwGRuPO7LH+b9z0EnTd 4Z5dMSOvUrQmHhP7JAcOU1e+ArXmA64MvoNyUFH0= Date: Mon, 31 Jan 2022 17:32:50 +0200 From: Laurent Pinchart To: Daniel Vetter Cc: Greg Kroah-Hartman , Stephen Boyd , Douglas Anderson , linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, "Rafael J. Wysocki" , Rob Clark , Russell King , Saravana Kannan Subject: Re: [PATCH v6 02/35] component: Introduce the aggregate bus_type Message-ID: References: <20220127200141.1295328-1-swboyd@chromium.org> <20220127200141.1295328-3-swboyd@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Daniel, On Mon, Jan 31, 2022 at 04:15:09PM +0100, Daniel Vetter wrote: > On Mon, Jan 31, 2022 at 2:48 PM Greg Kroah-Hartman wrote: > > On Thu, Jan 27, 2022 at 12:01:08PM -0800, Stephen Boyd wrote: > > > The component framework only provides 'bind' and 'unbind' callbacks to > > > tell the host driver that it is time to assemble the aggregate driver > > > now that all the components have probed. The component framework doesn't > > > attempt to resolve runtime PM or suspend/resume ordering, and explicitly > > > mentions this in the code. This lack of support leads to some pretty > > > gnarly usages of the 'prepare' and 'complete' power management hooks in > > > drivers that host the aggregate device, and it fully breaks down when > > > faced with ordering shutdown between the various components, the > > > aggregate driver, and the host driver that registers the whole thing. > > > > > > In a concrete example, the MSM display driver at drivers/gpu/drm/msm is > > > using 'prepare' and 'complete' to call the drm helpers > > > drm_mode_config_helper_suspend() and drm_mode_config_helper_resume() > > > respectively, so that it can move the aggregate driver suspend/resume > > > callbacks to be before and after the components that make up the drm > > > device call any suspend/resume hooks they have. This only works as long > > > as the component devices don't do anything in their own 'prepare' and > > > 'complete' callbacks. If they did, then the ordering would be incorrect > > > and we would be doing something in the component drivers before the > > > aggregate driver could do anything. Yuck! > > > > > > Similarly, when trying to add shutdown support to the MSM driver we run > > > across a problem where we're trying to shutdown the drm device via > > > drm_atomic_helper_shutdown(), but some of the devices in the encoder > > > chain have already been shutdown. This time, the component devices > > > aren't the problem (although they could be if they did anything in their > > > shutdown callbacks), but there's a DSI to eDP bridge in the encoder > > > chain that has already been shutdown before the driver hosting the > > > aggregate device runs shutdown. The ordering of driver probe is like > > > this: > > > > > > 1. msm_pdev_probe() (host driver) > > > 2. DSI bridge > > > 3. aggregate bind > > > > > > When it comes to shutdown we have this order: > > > > > > 1. DSI bridge > > > 2. msm_pdev_shutdown() (host driver) > > > > > > and so the bridge is already off, but we want to communicate to it to > > > turn things off on the display during msm_pdev_shutdown(). Double yuck! > > > Unfortunately, this time we can't split shutdown into multiple phases > > > and swap msm_pdev_shutdown() with the DSI bridge. > > > > > > Let's make the component_master_ops into an actual device driver that has > > > probe/remove/shutdown functions. The driver will only be bound to the > > > aggregate device once all component drivers have called component_add() > > > to indicate they're ready to assemble the aggregate driver. This allows > > > us to attach shutdown logic (and in the future runtime PM logic) to the > > > aggregate driver so that it runs the hooks in the correct order. > > > > I know I asked before, but I can not remember the answer. > > > > This really looks like it is turning into the aux bus code. Why can't > > you just use that instead here for this type of thing? You are creating > > another bus and drivers for that bus that are "fake" which is great, but > > that's what the aux bus code was supposed to help out with, so we > > wouldn't have to write more of these. > > > > So, if this really is different, can you document it here so I remember > > next time you resend this patch series? > > aux takes a device and splits it into a lot of sub-devices, each with > their own driver. > > This takes a pile of devices, and turns it into a single logical > device with a single driver. > > So aux is 1:N, component is N:1. > > And yes you asked this already, I typed this up already :-) That's clear, but I'm still not sure why we need a bus for this :-) I'm not very enthousiastic about that. Some of our problems come from the fact we need to coordinate many devices, adding new ones hardly seem to be a solution to that. Granted, the components framework doesn't work nicely, and is in dire need of love (and documentation), or possibly better a complete replacement. I'll try to review the series this week and see if alternatives would be possible. -- Regards, Laurent Pinchart