Message-ID: <20231130082343.GL3043@thinkpad>
Date:   Thu, 30 Nov 2023 13:53:43 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To:     mhi@...ts.linux.dev
Cc:     linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] bus: mhi: ep: Add support for interrupt moderation
 timer

On Thu, Oct 26, 2023 at 10:25:13AM +0530, Manivannan Sadhasivam wrote:
> MHI spec defines the interrupt moderation timer feature using which the
> host can limit the number of interrupts being raised for an event ring by
> the device. This feature allows the host to process multiple event ring
> elements by a single IRQ from the device, thereby eliminating the need to
> process IRQ for each element.
> 
> The INTMODT field in the event context array provides the value to be used
> for delaying the IRQ generation from device. This value, along with the
> Block Event Interrupt (BEI) flag of the TRE defines how IRQ is generated to
> the host.
> 
> Support for interrupt moderation timer is implemented using delayed
> workqueue in kernel. And a separate delayed work item is used for each
> event ring.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>

Applied to mhi-next!

- Mani

> ---
> 
> Changes in v2:
> 
> * Fixed the build issue reported by Kbuild bot.
> 
>  drivers/bus/mhi/ep/internal.h |  3 +++
>  drivers/bus/mhi/ep/main.c     | 22 +++++++++++++++++++---
>  drivers/bus/mhi/ep/ring.c     | 19 ++++++++++++++++++-
>  3 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/mhi/ep/internal.h b/drivers/bus/mhi/ep/internal.h
> index a2125fa5fe2f..8c5cf2b67951 100644
> --- a/drivers/bus/mhi/ep/internal.h
> +++ b/drivers/bus/mhi/ep/internal.h
> @@ -126,6 +126,7 @@ struct mhi_ep_ring {
>  	union mhi_ep_ring_ctx *ring_ctx;
>  	struct mhi_ring_element *ring_cache;
>  	enum mhi_ep_ring_type type;
> +	struct delayed_work intmodt_work;
>  	u64 rbase;
>  	size_t rd_offset;
>  	size_t wr_offset;
> @@ -135,7 +136,9 @@ struct mhi_ep_ring {
>  	u32 ch_id;
>  	u32 er_index;
>  	u32 irq_vector;
> +	u32 intmodt;
>  	bool started;
> +	bool irq_pending;
>  };
>  
>  struct mhi_ep_cmd {
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index e2513f5f47a6..c06111045a84 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -54,11 +54,27 @@ static int mhi_ep_send_event(struct mhi_ep_cntrl *mhi_cntrl, u32 ring_idx,
>  	mutex_unlock(&mhi_cntrl->event_lock);
>  
>  	/*
> -	 * Raise IRQ to host only if the BEI flag is not set in TRE. Host might
> -	 * set this flag for interrupt moderation as per MHI protocol.
> +	 * As per the MHI specification, section 4.3, Interrupt moderation:
> +	 *
> +	 * 1. If BEI flag is not set, cancel any pending intmodt work if started
> +	 * for the event ring and raise IRQ immediately.
> +	 *
> +	 * 2. If both BEI and intmodt are set, and if no IRQ is pending for the
> +	 * same event ring, start the IRQ delayed work as per the value of
> +	 * intmodt. If previous IRQ is pending, then do nothing as the pending
> +	 * IRQ is enough for the host to process the current event ring element.
> +	 *
> +	 * 3. If BEI is set and intmodt is not set, no need to raise IRQ.
>  	 */
> -	if (!bei)
> +	if (!bei) {
> +		if (READ_ONCE(ring->irq_pending))
> +			cancel_delayed_work(&ring->intmodt_work);
> +
>  		mhi_cntrl->raise_irq(mhi_cntrl, ring->irq_vector);
> +	} else if (ring->intmodt && !READ_ONCE(ring->irq_pending)) {
> +		WRITE_ONCE(ring->irq_pending, true);
> +		schedule_delayed_work(&ring->intmodt_work, msecs_to_jiffies(ring->intmodt));
> +	}
>  
>  	return 0;
>  
> diff --git a/drivers/bus/mhi/ep/ring.c b/drivers/bus/mhi/ep/ring.c
> index 115518ec76a4..a1071c13761b 100644
> --- a/drivers/bus/mhi/ep/ring.c
> +++ b/drivers/bus/mhi/ep/ring.c
> @@ -157,6 +157,15 @@ void mhi_ep_ring_init(struct mhi_ep_ring *ring, enum mhi_ep_ring_type type, u32
>  	}
>  }
>  
> +static void mhi_ep_raise_irq(struct work_struct *work)
> +{
> +	struct mhi_ep_ring *ring = container_of(work, struct mhi_ep_ring, intmodt_work.work);
> +	struct mhi_ep_cntrl *mhi_cntrl = ring->mhi_cntrl;
> +
> +	mhi_cntrl->raise_irq(mhi_cntrl, ring->irq_vector);
> +	WRITE_ONCE(ring->irq_pending, false);
> +}
> +
>  int mhi_ep_ring_start(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_ring *ring,
>  			union mhi_ep_ring_ctx *ctx)
>  {
> @@ -173,8 +182,13 @@ int mhi_ep_ring_start(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_ring *ring,
>  	if (ring->type == RING_TYPE_CH)
>  		ring->er_index = le32_to_cpu(ring->ring_ctx->ch.erindex);
>  
> -	if (ring->type == RING_TYPE_ER)
> +	if (ring->type == RING_TYPE_ER) {
>  		ring->irq_vector = le32_to_cpu(ring->ring_ctx->ev.msivec);
> +		ring->intmodt = FIELD_GET(EV_CTX_INTMODT_MASK,
> +					  le32_to_cpu(ring->ring_ctx->ev.intmod));
> +
> +		INIT_DELAYED_WORK(&ring->intmodt_work, mhi_ep_raise_irq);
> +	}
>  
>  	/* During ring init, both rp and wp are equal */
>  	memcpy_fromio(&val, (void __iomem *) &ring->ring_ctx->generic.rp, sizeof(u64));
> @@ -201,6 +215,9 @@ int mhi_ep_ring_start(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_ring *ring,
>  
>  void mhi_ep_ring_reset(struct mhi_ep_cntrl *mhi_cntrl, struct mhi_ep_ring *ring)
>  {
> +	if (ring->type == RING_TYPE_ER)
> +		cancel_delayed_work_sync(&ring->intmodt_work);
> +
>  	ring->started = false;
>  	kfree(ring->ring_cache);
>  	ring->ring_cache = NULL;
> 
> base-commit: 12606ba1d46b34a241eb3d0956727e5379f0f626
> -- 
> 2.25.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists