Re: [PATCH] spi/pl022: Add high priority message pump support
On Thu, Jan 26, 2012 at 7:48 AM, Linus Walleij wrote: > On Wed, Jan 25, 2012 at 3:02 PM, Mark Brown wrote: >> On Tue, Jan 24, 2012 at 10:14:32PM +0100, Linus Walleij wrote: > >>> This switches the PL022 worker to a kthread in order to get >>> hold of a mechanism to control the message pump priority. On >>> low-latency systems elevating the message kthread to realtime >>> priority give a real sleek response curve. This has been >>> confirmed by measurements. Realtime priority elevation for >>> a certain PL022 port can be requested from platform data. >> >> It really feels like we should be pulling this into the core - lots of >> drivers use a workqueue to drive data through the system and they're all >> going to have exactly the same issue. > > That reads to me like the entire message queue and "transfer pump" > mechanism from the PL022 driver should be made into generic > code. That is the key ingredient from the PL022 driver that has > allowed us to get real nice throughput on it. > > And that observation is correct, but a bit of upfront code. > > If Grant is in on it I might give it a try. Go for it and see what it looks like. g. -- Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/pl022: Add high priority message pump support
On Thu, Jan 26, 2012 at 03:48:59PM +0100, Linus Walleij wrote: > On Wed, Jan 25, 2012 at 3:02 PM, Mark Brown wrote: > > It really feels like we should be pulling this into the core - lots of > > drivers use a workqueue to drive data through the system and they're all > > going to have exactly the same issue. > That reads to me like the entire message queue and "transfer pump" > mechanism from the PL022 driver should be made into generic > code. That is the key ingredient from the PL022 driver that has > allowed us to get real nice throughput on it. > And that observation is correct, but a bit of upfront code. Probably, yes - lots of drivers seem to have a workqueue of some kind that drives the actual transfers and I strongly suspect that there's a lot of generality there. I have to confess that I had just thought that "transfer pump" was an obscure bit of jargon in the changelog so it's possible it's doing something device specific but it'd seem surprising TBH. > I would make it an opt-in for SPI drivers to have a generic > message queue instead of treating messages in a one-by-one > manner. That'd certainly be nice - where things use a workqueue they do often just pass the entire request from the caller off to it in one fell swoop so that sounds compatible. -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/pl022: Add high priority message pump support
On Wed, Jan 25, 2012 at 3:02 PM, Mark Brown wrote: > On Tue, Jan 24, 2012 at 10:14:32PM +0100, Linus Walleij wrote: >> This switches the PL022 worker to a kthread in order to get >> hold of a mechanism to control the message pump priority. On >> low-latency systems elevating the message kthread to realtime >> priority give a real sleek response curve. This has been >> confirmed by measurements. Realtime priority elevation for >> a certain PL022 port can be requested from platform data. > > It really feels like we should be pulling this into the core - lots of > drivers use a workqueue to drive data through the system and they're all > going to have exactly the same issue. That reads to me like the entire message queue and "transfer pump" mechanism from the PL022 driver should be made into generic code. That is the key ingredient from the PL022 driver that has allowed us to get real nice throughput on it. And that observation is correct, but a bit of upfront code. If Grant is in on it I might give it a try. I would make it an opt-in for SPI drivers to have a generic message queue instead of treating messages in a one-by-one manner. So say I add to spi/Kconfig that SPI_MASTER:s can define SPI_MASTER_QUEUE then add a spi_alloc_queued_master() call with some new #ifdef SPI_MASTER_QUEUE functions and fields added to in the struct spi_master to handle this stuff, and compile in the message pump so that these drivers does not implement the .transfer() call, but instead some more fine-grained ones would that fit the bill? Yours, Linus Walleij -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/pl022: Add high priority message pump support
On Tue, Jan 24, 2012 at 10:14:32PM +0100, Linus Walleij wrote: > From: Chris Blair > > This switches the PL022 worker to a kthread in order to get > hold of a mechanism to control the message pump priority. On > low-latency systems elevating the message kthread to realtime > priority give a real sleek response curve. This has been > confirmed by measurements. Realtime priority elevation for > a certain PL022 port can be requested from platform data. It really feels like we should be pulling this into the core - lots of drivers use a workqueue to drive data through the system and they're all going to have exactly the same issue. -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] spi/pl022: Add high priority message pump support
On 1/25/2012 2:44 AM, Linus WALLEIJ wrote:
> From: Chris Blair
>
> This switches the PL022 worker to a kthread in order to get
> hold of a mechanism to control the message pump priority. On
> low-latency systems elevating the message kthread to realtime
> priority give a real sleek response curve. This has been
> confirmed by measurements. Realtime priority elevation for
> a certain PL022 port can be requested from platform data.
>
> Signed-off-by: Chris Blair
> Signed-off-by: Linus Walleij
> ---
> drivers/spi/spi-pl022.c| 77 +--
> include/linux/amba/pl022.h |3 ++
> 2 files changed, 55 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
> index 2f9cb43..81847c9 100644
> --- a/drivers/spi/spi-pl022.c
> +++ b/drivers/spi/spi-pl022.c
> @@ -29,7 +29,7 @@
> #include
> #include
> #include
> -#include
> +#include
> #include
> #include
> #include
> @@ -41,6 +41,7 @@
> #include
> #include
> #include
> +#include
>
> /*
> * This macro is used to define some register default values.
> @@ -330,12 +331,13 @@ struct vendor_data {
> * @clk: outgoing clock "SPICLK" for the SPI bus
> * @master: SPI framework hookup
> * @master_info: controller-specific data from machine setup
> - * @workqueue: a workqueue on which any spi_message request is queued
> - * @pump_messages: work struct for scheduling work to the workqueue
> + * @kworker: thread struct for message pump
> + * @kworker_task: pointer to task for message pump kworker thread
> + * @pump_messages: work struct for scheduling work to the message pump
> * @queue_lock: spinlock to syncronise access to message queue
> * @queue: message queue
> - * @busy: workqueue is busy
> - * @running: workqueue is running
> + * @busy: message pump is busy
> + * @running: message pump is running
> * @pump_transfers: Tasklet used in Interrupt Transfer mode
> * @cur_msg: Pointer to current spi_message being processed
> * @cur_transfer: Pointer to current spi_transfer
> @@ -365,9 +367,10 @@ struct pl022 {
> struct clk *clk;
> struct spi_master *master;
> struct pl022_ssp_controller *master_info;
> - /* Driver message queue */
> - struct workqueue_struct *workqueue;
> - struct work_struct pump_messages;
> + /* Driver message pump */
> + struct kthread_worker kworker;
> + struct task_struct *kworker_task;
> + struct kthread_work pump_messages;
> spinlock_t queue_lock;
> struct list_headqueue;
> boolbusy;
> @@ -504,7 +507,7 @@ static void giveback(struct pl022 *pl022)
> pl022->cur_msg = NULL;
> pl022->cur_transfer = NULL;
> pl022->cur_chip = NULL;
> - queue_work(pl022->workqueue, &pl022->pump_messages);
> + queue_kthread_work(&pl022->kworker, &pl022->pump_messages);
> spin_unlock_irqrestore(&pl022->queue_lock, flags);
>
> msg->state = NULL;
> @@ -1494,8 +1497,8 @@ out:
> }
>
> /**
> - * pump_messages - Workqueue function which processes spi message queue
> - * @data: pointer to private data of SSP driver
> + * pump_messages - kthread work function which processes spi message queue
> + * @work: pointer to kthread work struct contained in the pl022 private
> struct
> *
> * This function checks if there is any spi message in the queue that
> * needs processing and delegate control to appropriate function
> @@ -1503,7 +1506,7 @@ out:
> * based on the kind of the transfer
> *
> */
> -static void pump_messages(struct work_struct *work)
> +static void pump_messages(struct kthread_work *work)
> {
> struct pl022 *pl022 =
> container_of(work, struct pl022, pump_messages);
> @@ -1556,7 +1559,7 @@ static void pump_messages(struct work_struct *work)
> if (!was_busy)
> /*
>* We enable the core voltage and clocks here, then the clocks
> - * and core will be disabled when this workqueue is run again
> + * and core will be disabled when this thread is run again
>* and there is no more work to be done.
>*/
> pm_runtime_get_sync(&pl022->adev->dev);
> @@ -1572,6 +1575,8 @@ static void pump_messages(struct work_struct *work)
>
> static int __init init_queue(struct pl022 *pl022)
> {
> + struct sched_param param = { .sched_priority = MAX_RT_PRIO - 1 };
> +
> INIT_LIST_HEAD(&pl022->queue);
> spin_lock_init(&pl022->queue_lock);
>
> @@ -1581,11 +1586,29 @@ static int __init init_queue(struct pl022 *pl022)
> tasklet_init(&pl022->pump_transfers, pump_transfers,
> (unsigned long)pl022);
>
> - INIT_WORK(&pl022->pump_messages, pump_messages);
> - pl022->workqueue = create_singlethread_workqueue(
> + init_kthread_wor
[PATCH] spi/pl022: Add high priority message pump support
From: Chris Blair
This switches the PL022 worker to a kthread in order to get
hold of a mechanism to control the message pump priority. On
low-latency systems elevating the message kthread to realtime
priority give a real sleek response curve. This has been
confirmed by measurements. Realtime priority elevation for
a certain PL022 port can be requested from platform data.
Signed-off-by: Chris Blair
Signed-off-by: Linus Walleij
---
drivers/spi/spi-pl022.c| 77 +--
include/linux/amba/pl022.h |3 ++
2 files changed, 55 insertions(+), 25 deletions(-)
diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 2f9cb43..81847c9 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -29,7 +29,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
/*
* This macro is used to define some register default values.
@@ -330,12 +331,13 @@ struct vendor_data {
* @clk: outgoing clock "SPICLK" for the SPI bus
* @master: SPI framework hookup
* @master_info: controller-specific data from machine setup
- * @workqueue: a workqueue on which any spi_message request is queued
- * @pump_messages: work struct for scheduling work to the workqueue
+ * @kworker: thread struct for message pump
+ * @kworker_task: pointer to task for message pump kworker thread
+ * @pump_messages: work struct for scheduling work to the message pump
* @queue_lock: spinlock to syncronise access to message queue
* @queue: message queue
- * @busy: workqueue is busy
- * @running: workqueue is running
+ * @busy: message pump is busy
+ * @running: message pump is running
* @pump_transfers: Tasklet used in Interrupt Transfer mode
* @cur_msg: Pointer to current spi_message being processed
* @cur_transfer: Pointer to current spi_transfer
@@ -365,9 +367,10 @@ struct pl022 {
struct clk *clk;
struct spi_master *master;
struct pl022_ssp_controller *master_info;
- /* Driver message queue */
- struct workqueue_struct *workqueue;
- struct work_struct pump_messages;
+ /* Driver message pump */
+ struct kthread_worker kworker;
+ struct task_struct *kworker_task;
+ struct kthread_work pump_messages;
spinlock_t queue_lock;
struct list_headqueue;
boolbusy;
@@ -504,7 +507,7 @@ static void giveback(struct pl022 *pl022)
pl022->cur_msg = NULL;
pl022->cur_transfer = NULL;
pl022->cur_chip = NULL;
- queue_work(pl022->workqueue, &pl022->pump_messages);
+ queue_kthread_work(&pl022->kworker, &pl022->pump_messages);
spin_unlock_irqrestore(&pl022->queue_lock, flags);
msg->state = NULL;
@@ -1494,8 +1497,8 @@ out:
}
/**
- * pump_messages - Workqueue function which processes spi message queue
- * @data: pointer to private data of SSP driver
+ * pump_messages - kthread work function which processes spi message queue
+ * @work: pointer to kthread work struct contained in the pl022 private struct
*
* This function checks if there is any spi message in the queue that
* needs processing and delegate control to appropriate function
@@ -1503,7 +1506,7 @@ out:
* based on the kind of the transfer
*
*/
-static void pump_messages(struct work_struct *work)
+static void pump_messages(struct kthread_work *work)
{
struct pl022 *pl022 =
container_of(work, struct pl022, pump_messages);
@@ -1556,7 +1559,7 @@ static void pump_messages(struct work_struct *work)
if (!was_busy)
/*
* We enable the core voltage and clocks here, then the clocks
-* and core will be disabled when this workqueue is run again
+* and core will be disabled when this thread is run again
* and there is no more work to be done.
*/
pm_runtime_get_sync(&pl022->adev->dev);
@@ -1572,6 +1575,8 @@ static void pump_messages(struct work_struct *work)
static int __init init_queue(struct pl022 *pl022)
{
+ struct sched_param param = { .sched_priority = MAX_RT_PRIO - 1 };
+
INIT_LIST_HEAD(&pl022->queue);
spin_lock_init(&pl022->queue_lock);
@@ -1581,11 +1586,29 @@ static int __init init_queue(struct pl022 *pl022)
tasklet_init(&pl022->pump_transfers, pump_transfers,
(unsigned long)pl022);
- INIT_WORK(&pl022->pump_messages, pump_messages);
- pl022->workqueue = create_singlethread_workqueue(
+ init_kthread_worker(&pl022->kworker);
+ pl022->kworker_task = kthread_run(kthread_worker_fn,
+ &pl022->kworker,
dev_name(pl022->master->dev.parent))
