Re: [PATCH v7 5/7] PCI: qcom: Handle MSIs routed to multiple GIC interrupts

From: Rob Herring <robh@kernel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Jingoo Han <jingoohan1@gmail.com>,
	Gustavo Pimentel <gustavo.pimentel@synopsys.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Stanimir Varbanov <svarbanov@mm-sol.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Vinod Koul <vkoul@kernel.org>,
	linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v7 5/7] PCI: qcom: Handle MSIs routed to multiple GIC interrupts
Date: Mon, 9 May 2022 16:00:03 -0500	[thread overview]
Message-ID: <YnmA02t7BBIPSgnY@robh.at.kernel.org> (raw)
In-Reply-To: <b334a2e6-69ae-690d-8560-25f8a1319e5c@linaro.org>

On Fri, May 06, 2022 at 10:40:56AM +0300, Dmitry Baryshkov wrote:
> On 06/05/2022 00:26, Rob Herring wrote:
> > On Thu, May 05, 2022 at 04:54:05PM +0300, Dmitry Baryshkov wrote:
> > > On some of Qualcomm platforms each group of 32 MSI vectors is routed to the
> > > separate GIC interrupt. Thus to receive higher MSI vectors properly,
> > > add separate msi_host_init()/msi_host_deinit() handling additional host
> > > IRQs.
> > 
> > msi_host_init() has 1 user (keystone) as it doesn't use the DWC MSI
> > controller. But QCom does given the access to PCIE_MSI_INTR0_STATUS,
> > so mutiple MSI IRQ outputs must have been added in newer versions of the
> > DWC IP. If so, it's only a matter of time for another platform to
> > do the same thing. Maybe someone from Synopsys could confirm?
> 
> This is a valid question, and if you check, first iterations of this
> patchset had this in the dwc core ([1], [2]). Exactly for the reason this
> might be usable for other platforms.
> 
> Then I did some research for other platforms using DWC PCIe IP core. For
> example, both Tegra Xavier and iMX6 support up to 256 MSI vectors, they use
> DWC MSI IRQ controller. The iMX6 TRM explicitly describes using different
> MSI groups for different endpoints. The diagram shows 8 MSI IRQ signal
> lines. However in the end the signals from all groups are OR'ed to form a
> single host msi_ctrl_int. Thus currently I suppose that using multiple MSI
> IRQs is a peculiarity of Qualcomm platform.

Chip integration very often will just OR together interrupts or not. 
It's completely at the whim of the SoC design, so I'd say both cases are 
very likely. Seems to be a feature in newer versions of the IP. Probably 
requested by some misguided h/w person thinking split interrupts would 
be 'faster'. (Sorry, too many past discussions with h/w designers on 
this topic.)

> > Therefore this should all be handled in the DWC core. In general, I
> > don't want to see more users nor more ops if we don't have to. Let's not
> > create ops for what can be handled as data. AFAICT, this is just number
> > of MSIs and # of MSIs per IRQ. It seems plausible another platform could
> > do something similar and supporting it in the core code wouldn't
> > negatively impact other platforms.
> 
> I wanted to balance adding additional ops vs complicating the core for other
> platforms. And I still suppose that platform specifics should go to the
> platform driver. However if you prefer [1] and [2], we can go back to that
> implementation.

Yes, I prefer that implementation.

Rob