Re: [PATCH v12 6/8] PCI: dwc: Implement special ISR handler for split MSI IRQ setup

[Rob Herring <robh@kernel.org>]

On Mon, May 23, 2022 at 09:18:34PM +0300, Dmitry Baryshkov wrote:
> If the PCIe DWC controller uses split MSI IRQs for reporting MSI
> vectors, it is possible to detect, which group triggered the interrupt.
> Provide an optimized version of MSI ISR handler that will handle just a
> single MSI group instead of handling all of them.

A lot more complexity to save 7 register reads...

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../pci/controller/dwc/pcie-designware-host.c | 86 ++++++++++++++-----
>  1 file changed, 65 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 98a57249ecaf..2b2de517301a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -52,34 +52,42 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
>  	.chip	= &dw_pcie_msi_irq_chip,
>  };
>  
> +static inline irqreturn_t dw_handle_single_msi_group(struct pcie_port *pp, int i)
> +{
> +	int pos;
> +	unsigned long val;
> +	u32 status;
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +
> +	status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> +				   (i * MSI_REG_CTRL_BLOCK_SIZE));
> +	if (!status)
> +		return IRQ_NONE;
> +
> +	val = status;
> +	pos = 0;
> +	while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
> +				    pos)) != MAX_MSI_IRQS_PER_CTRL) {

for_each_set_bit() doesn't work here?

> +		generic_handle_domain_irq(pp->irq_domain,
> +					  (i * MAX_MSI_IRQS_PER_CTRL) +
> +					  pos);
> +		pos++;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
>  /* MSI int handler */
>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  {
> -	int i, pos;
> -	unsigned long val;
> -	u32 status, num_ctrls;
> +	int i;
> +	u32 num_ctrls;
>  	irqreturn_t ret = IRQ_NONE;
> -	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  
>  	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
>  
> -	for (i = 0; i < num_ctrls; i++) {
> -		status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> -					   (i * MSI_REG_CTRL_BLOCK_SIZE));
> -		if (!status)
> -			continue;
> -
> -		ret = IRQ_HANDLED;
> -		val = status;
> -		pos = 0;
> -		while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
> -					    pos)) != MAX_MSI_IRQS_PER_CTRL) {
> -			generic_handle_domain_irq(pp->irq_domain,
> -						  (i * MAX_MSI_IRQS_PER_CTRL) +
> -						  pos);
> -			pos++;
> -		}
> -	}
> +	for (i = 0; i < num_ctrls; i++)
> +		ret |= dw_handle_single_msi_group(pp, i);
>  
>  	return ret;
>  }
> @@ -98,6 +106,38 @@ static void dw_chained_msi_isr(struct irq_desc *desc)
>  	chained_irq_exit(chip, desc);
>  }
>  
> +static void dw_split_msi_isr(struct irq_desc *desc)
> +{
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	int irq = irq_desc_get_irq(desc);
> +	struct pcie_port *pp;
> +	int i;
> +	u32 num_ctrls;
> +	struct dw_pcie *pci;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	pp = irq_desc_get_handler_data(desc);
> +	pci = to_dw_pcie_from_pp(pp);
> +
> +	/*
> +	 * Unlike generic dw_handle_msi_irq(), we can determine which group of
> +	 * MSIs triggered the IRQ, so process just that group.
> +	 */
> +	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +
> +	for (i = 0; i < num_ctrls; i++) {
> +		if (pp->msi_irq[i] == irq) {
> +			dw_handle_single_msi_group(pp, i);
> +			break;
> +		}
> +	}
> +
> +	WARN_ON_ONCE(i == num_ctrls);
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
>  static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
>  {
>  	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> @@ -336,6 +376,7 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  	struct platform_device *pdev = to_platform_device(dev);
>  	int ret;
>  	u32 ctrl, num_ctrls;
> +	bool has_split_msi_irq = false;
>  
>  	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++)
>  		pp->irq_mask[ctrl] = ~0;
> @@ -344,6 +385,8 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  		ret = dw_pcie_parse_split_msi_irq(pp);
>  		if (ret < 0 && ret != -ENXIO)
>  			return ret;
> +		else if (!ret)
> +			has_split_msi_irq = true;
>  	}
>  
>  	if (!pp->num_vectors)
> @@ -372,6 +415,7 @@ static int dw_pcie_msi_host_init(struct pcie_port *pp)
>  	for (ctrl = 0; ctrl < num_ctrls; ctrl++)
>  		if (pp->msi_irq[ctrl] > 0)
>  			irq_set_chained_handler_and_data(pp->msi_irq[ctrl],
> +							 has_split_msi_irq ? dw_split_msi_isr :
>  							 dw_chained_msi_isr,
>  							 pp);
>  
> -- 
> 2.35.1
>