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 B21E4C6FA8A for ; Tue, 13 Sep 2022 07:18:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231195AbiIMHSq (ORCPT ); Tue, 13 Sep 2022 03:18:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48454 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231174AbiIMHS0 (ORCPT ); Tue, 13 Sep 2022 03:18:26 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0B08B4BD30; Tue, 13 Sep 2022 00:18:22 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 9EF8AB80DC2; Tue, 13 Sep 2022 07:18:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4B9A0C433B5; Tue, 13 Sep 2022 07:18:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1663053499; bh=FC9a9cUyZ6tKHf4RlLIdqtcJUM8pTZqYprxyD5qXv7Q=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=S3A9nfZgG47jTHHLHmXmbPEuZC2AludnQHnTqUXcqG150oeHp4MzYpHZ5QEnaEe+J hOMFjatPLS5ji11A3qfoG1B5R7Mh6j4u2oHbpctM5uFWbijuq8ECMmmsDzUBwCEvol zppyt6HorKW/b5mwvMgdzcIkGK90dajcR97tB/oxo/AF21knSWrJToJ12WRTN0DQy/ BnMGbc+AdwBBmBQLJFLVFRYO6WpA/ihe1nSbBbJIvdjykxVoKcI7rXQDGd66XXWEES 4fXU9zP4Ez9pI4X1G0hr4ejXDyMBZP4cMZjoo+OTrLAqSbfAnb5x1U1UjxLuACw3YB a2f9UKGgifaGQ== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from ) id 1oY0BV-0000lx-1z; Tue, 13 Sep 2022 09:18:17 +0200 Date: Tue, 13 Sep 2022 09:18:17 +0200 From: Johan Hovold To: Doug Anderson Cc: Dmitry Baryshkov , Johan Hovold , Rob Clark , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , David Airlie , Daniel Vetter , Sean Paul , Stephen Boyd , Bjorn Andersson , Manivannan Sadhasivam , dri-devel , linux-arm-msm , freedreno , LKML , "# 4.0+" Subject: Re: [PATCH 4/7] drm/msm/dp: fix aux-bus EP lifetime Message-ID: References: <20220912154046.12900-1-johan+linaro@kernel.org> <20220912154046.12900-5-johan+linaro@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 13, 2022 at 07:35:15AM +0100, Doug Anderson wrote: > Hi, > > On Mon, Sep 12, 2022 at 7:10 PM Dmitry Baryshkov > wrote: > > > > On 12/09/2022 18:40, Johan Hovold wrote: > > > Device-managed resources allocated post component bind must be tied to > > > the lifetime of the aggregate DRM device or they will not necessarily be > > > released when binding of the aggregate device is deferred. > > > > > > This can lead resource leaks or failure to bind the aggregate device > > > when binding is later retried and a second attempt to allocate the > > > resources is made. > > > > > > For the DP aux-bus, an attempt to populate the bus a second time will > > > simply fail ("DP AUX EP device already populated"). > > > > > > Fix this by amending the DP aux interface and tying the lifetime of the > > > EP device to the DRM device rather than DP controller platform device. > > > > Doug, could you please take a look? > > > > For me this is another reminder/pressure point that we should populate > > the AUX BUS from the probe(), before binding the components together. > > Aside from the kernel robot complaints, I'm not necessarily convinced. > I think we know that the AUX DP stuff in MSM-DP is fragile right now > and Qualcomm has promised to clean it up. This really feels like a > band-aid and is really a sign that we're populating the AUX DP bus in > the wrong place in Qualcomm's code. As you said, if we moved this to > probe(), which is the plan in the promised cleanup, then it wouldn't > be a problem. Right, but that appears to be non-trivial judging from the discussions you had back when the offending patch was merged: https://lore.kernel.org/lkml/CAD=FV=X+QvjwoT2zGP82KW4kD0oMUY6ZgCizSikNX_Uj8dNDqA@mail.gmail.com/t/#u > As far as I know Qualcomm has queued this cleanup behind their current > PSR work (though it's never been clear why both can't be worked on at > the same time) and the PSR work was stalled because they couldn't > figure out what caused the glitching I reported. It's still on my nag > list that I bring up with them every week... > > In any case, if a band-aid is urgent, maybe you could just call > of_dp_aux_populate_bus() directly in Qualcomm code and you could add > your own devm_add_action_or_reset() on the DRM device. Yeah, that's probably better. I apparently missed a bunch of users of devm_of_dp_aux_populate_ep_devices() after searching for devm_of_dp_aux_populate_bus() instead. Judging from a quick glance all of these populate the bus at probe, so Qualcomm indeed appears to be the odd bird here. But the bug is real, in mainline and needs to be fixed, so rolling a custom devm action indeed should to be the right thing to do here (if only to have a smaller fix). Johan