linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v5] bus: mhi: core: Add support for processing priority of event ring
       [not found]   ` <9300cf49a498521f471d6106131bd675@codeaurora.org>
@ 2021-07-29  0:06     ` Bhaumik Bhatt
  2021-09-16  5:28       ` Kalle Valo
  0 siblings, 1 reply; 2+ messages in thread
From: Bhaumik Bhatt @ 2021-07-29  0:06 UTC (permalink / raw)
  To: kvalo, ath11k
  Cc: linux-arm-msm, hemantk, linux-kernel, Manivannan Sadhasivam,
	linux-wireless

Hi Kalle,

On 2021-07-16 11:22 AM, Bhaumik Bhatt wrote:
> Hi Mani,
> 
> On 2021-07-16 04:49 AM, Manivannan Sadhasivam wrote:
>> On Fri, Jun 25, 2021 at 10:22:08AM -0700, Bhaumik Bhatt wrote:
>>> From: Hemant Kumar <hemantk@codeaurora.org>
>>> 
>>> Event ring priorities are currently set to 1 and are unused.
>>> Default processing priority for event rings is set to regular
>>> tasklet. Controllers can choose to use high priority tasklet
>>> scheduling for certain event rings critical for processing such
>>> as ones transporting control information if they wish to avoid
>>> system scheduling delays for those packets. In order to support
>>> these use cases, allow controllers to set event ring priority to
>>> high.
>>> 
>> 
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> 
>> Just curious, what are the event rings you are going to make as high
>> priority? If you are going to do that for existing controllers, please
>> submit a patch now itself.
>> 
>> Thanks,
>> Mani
>> 
> Idea for this patch came from 914b72a6948b ("bus: mhi: Wait for M2
> state during system resume").
> 
> If WLAN ath11k controller driver wants to avoid the scenario mentioned 
> in
> the above patch, it will help them to have a high priority for their 
> dedicated
> control events ring.
> 
> I would defer to Kalle and others in ath11k, whether or not they are
> OK to take that route.
> as an update to priority will just help return from resume faster.
> It will also depend on system load/reproducibility rate of the 
> scenario.
> 
> I can provide a patch for them to review/test since I do not have the
> setup for it.
> 
Would you like to try this patch out? It basically increases the 
priority
at which the control events for M0/M1/M3 state changes are handled.

Let me know if you have any questions.

>>> Signed-off-by: Hemant Kumar <hemantk@codeaurora.org>
>>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>>> ---
>>> v5:
>>> -Add controller changes and enable usage of event ring configuration 
>>> priorities
>>> -Fix nitpick, use high instead of hi in kdoc
>>> 
>>> v4:
>>> -Update fixed priority for all events to default to fix bug in v3
>>> -Supply changelog
>>> 
>>> v3:
>>> -Revert to enum approach
>>> -Use 0 as default and 1 as high in enum
>>> -Do not use config values for event rings
>>> 
>>> v2:
>>> -Use boolean approach for easy maintenance as controllers do not need 
>>> updates
>>> 
>>> 
>>> 
>>>  drivers/bus/mhi/core/init.c           |  3 +-
>>>  drivers/bus/mhi/core/internal.h       |  2 +-
>>>  drivers/bus/mhi/core/main.c           | 19 ++++++++--
>>>  drivers/bus/mhi/pci_generic.c         | 66 
>>> +++++++++++++++++------------------
>>>  drivers/net/wireless/ath/ath11k/mhi.c |  4 +--
>>>  include/linux/mhi.h                   | 14 ++++++--
>>>  6 files changed, 65 insertions(+), 43 deletions(-)
>>> 
>>> diff --git a/drivers/bus/mhi/core/init.c 
>>> b/drivers/bus/mhi/core/init.c
>>> index c81b377..4446760 100644
>>> --- a/drivers/bus/mhi/core/init.c
>>> +++ b/drivers/bus/mhi/core/init.c
>>> @@ -673,8 +673,7 @@ static int parse_ev_cfg(struct mhi_controller 
>>> *mhi_cntrl,
>>>  				&mhi_cntrl->mhi_chan[mhi_event->chan];
>>>  		}
>>> 
>>> -		/* Priority is fixed to 1 for now */
>>> -		mhi_event->priority = 1;
>>> +		mhi_event->priority = event_cfg->priority;
>>> 
>>>  		mhi_event->db_cfg.brstmode = event_cfg->mode;
>>>  		if (MHI_INVALID_BRSTMODE(mhi_event->db_cfg.brstmode))
>>> diff --git a/drivers/bus/mhi/core/internal.h 
>>> b/drivers/bus/mhi/core/internal.h
>>> index 672052f..666e102 100644
>>> --- a/drivers/bus/mhi/core/internal.h
>>> +++ b/drivers/bus/mhi/core/internal.h
>>> @@ -535,7 +535,7 @@ struct mhi_event {
>>>  	u32 intmod;
>>>  	u32 irq;
>>>  	int chan; /* this event ring is dedicated to a channel (optional) 
>>> */
>>> -	u32 priority;
>>> +	enum mhi_er_priority priority;
>>>  	enum mhi_er_data_type data_type;
>>>  	struct mhi_ring ring;
>>>  	struct db_cfg db_cfg;
>>> diff --git a/drivers/bus/mhi/core/main.c 
>>> b/drivers/bus/mhi/core/main.c
>>> index 8ac73f9..bfc9776 100644
>>> --- a/drivers/bus/mhi/core/main.c
>>> +++ b/drivers/bus/mhi/core/main.c
>>> @@ -425,10 +425,11 @@ void mhi_create_devices(struct mhi_controller 
>>> *mhi_cntrl)
>>>  	}
>>>  }
>>> 
>>> -irqreturn_t mhi_irq_handler(int irq_number, void *dev)
>>> +irqreturn_t mhi_irq_handler(int irq_number, void *priv)
>>>  {
>>> -	struct mhi_event *mhi_event = dev;
>>> +	struct mhi_event *mhi_event = priv;
>>>  	struct mhi_controller *mhi_cntrl = mhi_event->mhi_cntrl;
>>> +	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>>>  	struct mhi_event_ctxt *er_ctxt =
>>>  		&mhi_cntrl->mhi_ctxt->er_ctxt[mhi_event->er_index];
>>>  	struct mhi_ring *ev_ring = &mhi_event->ring;
>>> @@ -454,8 +455,20 @@ irqreturn_t mhi_irq_handler(int irq_number, void 
>>> *dev)
>>> 
>>>  		if (mhi_dev)
>>>  			mhi_notify(mhi_dev, MHI_CB_PENDING_DATA);
>>> -	} else {
>>> +
>>> +		return IRQ_HANDLED;
>>> +	}
>>> +
>>> +	switch (mhi_event->priority) {
>>> +	case MHI_ER_PRIORITY_HI:
>>> +		tasklet_hi_schedule(&mhi_event->task);
>>> +		break;
>>> +	case MHI_ER_PRIORITY_DEFAULT:
>>>  		tasklet_schedule(&mhi_event->task);
>>> +		break;
>>> +	default:
>>> +		dev_err(dev, "Skip event of unknown priority\n");
>>> +		break;
>>>  	}
>>> 
>>>  	return IRQ_HANDLED;
>>> diff --git a/drivers/bus/mhi/pci_generic.c 
>>> b/drivers/bus/mhi/pci_generic.c
>>> index 31360a2..5886547 100644
>>> --- a/drivers/bus/mhi/pci_generic.c
>>> +++ b/drivers/bus/mhi/pci_generic.c
>>> @@ -74,17 +74,17 @@ struct mhi_pci_dev_info {
>>>  		.doorbell_mode_switch = false,		\
>>>  	}
>>> 
>>> -#define MHI_EVENT_CONFIG_CTRL(ev_ring, el_count) \
>>> -	{					\
>>> -		.num_elements = el_count,	\
>>> -		.irq_moderation_ms = 0,		\
>>> -		.irq = (ev_ring) + 1,		\
>>> -		.priority = 1,			\
>>> -		.mode = MHI_DB_BRST_DISABLE,	\
>>> -		.data_type = MHI_ER_CTRL,	\
>>> -		.hardware_event = false,	\
>>> -		.client_managed = false,	\
>>> -		.offload_channel = false,	\
>>> +#define MHI_EVENT_CONFIG_CTRL(ev_ring, el_count)	\
>>> +	{						\
>>> +		.num_elements = el_count,		\
>>> +		.irq_moderation_ms = 0,			\
>>> +		.irq = (ev_ring) + 1,			\
>>> +		.priority = MHI_ER_PRIORITY_DEFAULT,	\
>>> +		.mode = MHI_DB_BRST_DISABLE,		\
>>> +		.data_type = MHI_ER_CTRL,		\
>>> +		.hardware_event = false,		\
>>> +		.client_managed = false,		\
>>> +		.offload_channel = false,		\
>>>  	}
>>> 
>>>  #define MHI_CHANNEL_CONFIG_HW_UL(ch_num, ch_name, el_count, ev_ring) 
>>> \
>>> @@ -177,31 +177,31 @@ struct mhi_pci_dev_info {
>>>  		.doorbell_mode_switch = false,		\
>>>  	}
>>> 
>>> -#define MHI_EVENT_CONFIG_DATA(ev_ring, el_count) \
>>> -	{					\
>>> -		.num_elements = el_count,	\
>>> -		.irq_moderation_ms = 5,		\
>>> -		.irq = (ev_ring) + 1,		\
>>> -		.priority = 1,			\
>>> -		.mode = MHI_DB_BRST_DISABLE,	\
>>> -		.data_type = MHI_ER_DATA,	\
>>> -		.hardware_event = false,	\
>>> -		.client_managed = false,	\
>>> -		.offload_channel = false,	\
>>> +#define MHI_EVENT_CONFIG_DATA(ev_ring, el_count)	\
>>> +	{						\
>>> +		.num_elements = el_count,		\
>>> +		.irq_moderation_ms = 5,			\
>>> +		.irq = (ev_ring) + 1,			\
>>> +		.priority = MHI_ER_PRIORITY_DEFAULT,	\
>>> +		.mode = MHI_DB_BRST_DISABLE,		\
>>> +		.data_type = MHI_ER_DATA,		\
>>> +		.hardware_event = false,		\
>>> +		.client_managed = false,		\
>>> +		.offload_channel = false,		\
>>>  	}
>>> 
>>>  #define MHI_EVENT_CONFIG_HW_DATA(ev_ring, el_count, ch_num) \
>>> -	{					\
>>> -		.num_elements = el_count,	\
>>> -		.irq_moderation_ms = 1,		\
>>> -		.irq = (ev_ring) + 1,		\
>>> -		.priority = 1,			\
>>> -		.mode = MHI_DB_BRST_DISABLE,	\
>>> -		.data_type = MHI_ER_DATA,	\
>>> -		.hardware_event = true,		\
>>> -		.client_managed = false,	\
>>> -		.offload_channel = false,	\
>>> -		.channel = ch_num,		\
>>> +	{						\
>>> +		.num_elements = el_count,		\
>>> +		.irq_moderation_ms = 1,			\
>>> +		.irq = (ev_ring) + 1,			\
>>> +		.priority = MHI_ER_PRIORITY_DEFAULT,	\
>>> +		.mode = MHI_DB_BRST_DISABLE,		\
>>> +		.data_type = MHI_ER_DATA,		\
>>> +		.hardware_event = true,			\
>>> +		.client_managed = false,		\
>>> +		.offload_channel = false,		\
>>> +		.channel = ch_num,			\
>>>  	}
>>> 
>>>  static const struct mhi_channel_config modem_qcom_v1_mhi_channels[] 
>>> = {
>>> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c 
>>> b/drivers/net/wireless/ath/ath11k/mhi.c
>>> index 27b394d..b7864fc 100644
>>> --- a/drivers/net/wireless/ath/ath11k/mhi.c
>>> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
>>> @@ -86,7 +86,7 @@ static struct mhi_event_config 
>>> ath11k_mhi_events_qca6390[] = {
>>>  		.irq_moderation_ms = 1,
>>>  		.irq = 2,
>>>  		.mode = MHI_DB_BRST_DISABLE,
>>> -		.priority = 1,
>>> +		.priority = MHI_ER_PRIORITY_DEFAULT,
>>>  		.hardware_event = false,
>>>  		.client_managed = false,
>>>  		.offload_channel = false,
>>> @@ -179,7 +179,7 @@ static struct mhi_event_config 
>>> ath11k_mhi_events_qcn9074[] = {
>>>  		.irq_moderation_ms = 1,
>>>  		.irq = 2,
>>>  		.mode = MHI_DB_BRST_DISABLE,
>>> -		.priority = 1,
>>> +		.priority = MHI_ER_PRIORITY_DEFAULT,
>>>  		.hardware_event = false,
>>>  		.client_managed = false,
>>>  		.offload_channel = false,
>>> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
>>> index 86cea52..3e92e85 100644
>>> --- a/include/linux/mhi.h
>>> +++ b/include/linux/mhi.h
>>> @@ -198,6 +198,16 @@ enum mhi_er_data_type {
>>>  };
>>> 
>>>  /**
>>> + * enum mhi_er_priority - Event ring processing priority
>>> + * @MHI_ER_PRIORITY_DEFAULT: processed by regular tasklet
>>> + * @MHI_ER_PRIORITY_HI: processed by high priority tasklet
>>> + */
>>> +enum mhi_er_priority {
>>> +	MHI_ER_PRIORITY_DEFAULT,
>>> +	MHI_ER_PRIORITY_HI,
>>> +};
>>> +
>>> +/**
>>>   * enum mhi_db_brst_mode - Doorbell mode
>>>   * @MHI_DB_BRST_DISABLE: Burst mode disable
>>>   * @MHI_DB_BRST_ENABLE: Burst mode enable
>>> @@ -250,7 +260,7 @@ struct mhi_channel_config {
>>>   * @irq_moderation_ms: Delay irq for additional events to be 
>>> aggregated
>>>   * @irq: IRQ associated with this ring
>>>   * @channel: Dedicated channel number. U32_MAX indicates a 
>>> non-dedicated ring
>>> - * @priority: Priority of this ring. Use 1 for now
>>> + * @priority: Processing priority of this ring.
>>>   * @mode: Doorbell mode
>>>   * @data_type: Type of data this ring will process
>>>   * @hardware_event: This ring is associated with hardware channels
>>> @@ -262,7 +272,7 @@ struct mhi_event_config {
>>>  	u32 irq_moderation_ms;
>>>  	u32 irq;
>>>  	u32 channel;
>>> -	u32 priority;
>>> +	enum mhi_er_priority priority;
>>>  	enum mhi_db_brst_mode mode;
>>>  	enum mhi_er_data_type data_type;
>>>  	bool hardware_event;
>>> --
>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>>> Forum,
>>> a Linux Foundation Collaborative Project
>>> 
> 
> Thanks,
> Bhaumik
> ---
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
> Forum,
> a Linux Foundation Collaborative Project

Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v5] bus: mhi: core: Add support for processing priority of event ring
  2021-07-29  0:06     ` [PATCH v5] bus: mhi: core: Add support for processing priority of event ring Bhaumik Bhatt
@ 2021-09-16  5:28       ` Kalle Valo
  0 siblings, 0 replies; 2+ messages in thread
From: Kalle Valo @ 2021-09-16  5:28 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: ath11k, linux-arm-msm, hemantk, linux-kernel,
	Manivannan Sadhasivam, linux-wireless

Hi Bhaumik,

Bhaumik Bhatt <bbhatt@codeaurora.org> writes:

> On 2021-07-16 11:22 AM, Bhaumik Bhatt wrote:
>> Hi Mani,
>>
>> On 2021-07-16 04:49 AM, Manivannan Sadhasivam wrote:
>>> On Fri, Jun 25, 2021 at 10:22:08AM -0700, Bhaumik Bhatt wrote:
>>>> From: Hemant Kumar <hemantk@codeaurora.org>
>>>>
>>>> Event ring priorities are currently set to 1 and are unused.
>>>> Default processing priority for event rings is set to regular
>>>> tasklet. Controllers can choose to use high priority tasklet
>>>> scheduling for certain event rings critical for processing such
>>>> as ones transporting control information if they wish to avoid
>>>> system scheduling delays for those packets. In order to support
>>>> these use cases, allow controllers to set event ring priority to
>>>> high.
>>>>
>>>
>>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>
>>> Just curious, what are the event rings you are going to make as high
>>> priority? If you are going to do that for existing controllers, please
>>> submit a patch now itself.
>>>
>>> Thanks,
>>> Mani
>>>
>> Idea for this patch came from 914b72a6948b ("bus: mhi: Wait for M2
>> state during system resume").
>>
>> If WLAN ath11k controller driver wants to avoid the scenario
>> mentioned in
>> the above patch, it will help them to have a high priority for their
>> dedicated
>> control events ring.
>>
>> I would defer to Kalle and others in ath11k, whether or not they are
>> OK to take that route.
>> as an update to priority will just help return from resume faster.
>> It will also depend on system load/reproducibility rate of the
>> scenario.
>>
>> I can provide a patch for them to review/test since I do not have the
>> setup for it.
>>
> Would you like to try this patch out? It basically increases the
> priority
> at which the control events for M0/M1/M3 state changes are handled.
>
> Let me know if you have any questions.

At the moment I'm seriously lagging with patches so I don't really have
any free time. Can some other ath11k developer help here?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-09-16  5:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1624641728-3886-1-git-send-email-bbhatt@codeaurora.org>
     [not found] ` <20210716114926.GH3323@workstation>
     [not found]   ` <9300cf49a498521f471d6106131bd675@codeaurora.org>
2021-07-29  0:06     ` [PATCH v5] bus: mhi: core: Add support for processing priority of event ring Bhaumik Bhatt
2021-09-16  5:28       ` Kalle Valo

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox