[Cbe-oss-dev] [RFC/PATCH] libspe2: use mapped mailbox registers to speed up mailbox communication

[Kazunori Asayama]

stenzel at de.ibm.com wrote:
> Thanks for the comments so far. I tried to integrate most of them.
> This patch replaces the previous one.
> 
> The following patch speeds up mailbox communication by
> accessing the mailbox registers directly via mapped problem
> state if the context was created with the SPE_MAP_PS flag.
> 
> Comments are still appreciated.

Here are my comments:

> +static __inline__ int _base_spe_out_mbox_read_ps(spe_context_ptr_t spectx,
> +                        unsigned int mbox_data[], 
> +                        int count)
> +{
> +	int rc;
> +	int entries;
> +
> +	volatile struct spe_spu_control_area *cntl_area =
> +        	spectx->base_private->cntl_mmap_base;
> +
> +	_base_spe_context_lock(spectx, FD_MBOX);
> +	rc = 0;
> +	while (rc < count) {
> +		entries = cntl_area->SPU_Mbox_Stat & 0xFF;
> +		if (entries == 0) break;  // no entries
> +		if (entries > count) entries = count; // don't read more than we need;

This line must be:

		if (entries > count - rc) entries = count - rc;

(however, this bug can't be produced if the depth of mbox is 1...)

(snip)

> +static __inline__ int _base_spe_in_mbox_write_ps(spe_context_ptr_t spectx,
> +                        unsigned int *mbox_data, 
> +                        int count)
> +{
> +	volatile struct spe_spu_control_area *cntl_area =
> +        	spectx->base_private->cntl_mmap_base;
> +	int total = 0;
> +	int i;

The variable 'i' is not used.

> +	unsigned int *aux;
> +	int space;
> +
> +	_base_spe_context_lock(spectx, FD_WBOX);
> +	aux = mbox_data;
> +	while (total < count) {
> +		space = (cntl_area->SPU_Mbox_Stat >> 8) & 0xFF;
> +		if (space == 0) break;  // no space
> +		if (space > count) space = count; // don't write more than we have

This line must be:

		if (space > count - total) space = count - total;

(snip)

> @@ -72,27 +129,63 @@ int _base_spe_in_mbox_write(spe_context_
>  	switch (behavior_flag) {
>  	case SPE_MBOX_ALL_BLOCKING: // write all, even if blocking
>  		total = rc = 0;
> -		while (total < 4*count) {
> -			rc = write(open_if_closed(spectx,FD_WBOX, 0),
> -					(const char *)mbox_data + total, 4*count - total);
> -			if (rc == -1) {
> -				break;
> +		if (spectx->base_private->flags & SPE_MAP_PS) {
> +			do {
> +				aux = mbox_data + total;
> +				total += _base_spe_in_mbox_write_ps(spectx, aux, count);

This line must be:

				total += _base_spe_in_mbox_write_ps(spectx, aux, count - total);

(snip)

> @@ -200,23 +311,39 @@ int _base_spe_out_intr_mbox_read(spe_con
>                          unsigned int data )
>  {
>  	int rc;
> +	volatile struct spe_spu_control_area *cntl_area =
> +        	spectx->base_private->cntl_mmap_base;

The variable cntl_area is not used.

--
(ASAYAMA Kazunori
  (asayama at sm.sony.co.jp))
t