All of lore.kernel.org
 help / color / mirror / Atom feed
From: josh.wu@atmel.com (Josh Wu)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] media: atmel-isi: increase the burst length to improve the performance
Date: Wed, 26 Nov 2014 10:54:48 +0800	[thread overview]
Message-ID: <547540F8.7040901@atmel.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1411252319430.17362@axis700.grange>

Hi, Gunenadi

On 11/26/2014 6:21 AM, Guennadi Liakhovetski wrote:
> Hi Josh,
>
> On Tue, 25 Nov 2014, Josh Wu wrote:
>
>> The burst length could be BEATS_4/8/16. Before this patch, isi use default
>> value BEATS_4. To imporve the performance we could set it to BEATS_16.
>>
>> Otherwise sometime it would cause the ISI overflow error.
> Without looking at datasheets - what does this bit do? Change the transfer
> length?
Atmel ISI has two internal 32-bytes FIFOs, one for Preview, another for 
Codec.
This field is the threshold to trigger the FIFO transfer to AHB by DMA.
We can set the threshold to allow 4-bytes, 8-bytes or 16-bytes for a burst.
BEATS_4, means only allow 4-bytes in a burst.
BEATS_8, means only allow 4-bytes and 8-bytes in a burst.
BEATS_16, means allow 4-bytes, 8-bytes and 16-bytes in a burst.

So BEATS_16 will use the DMA more efficient if has lots of data to 
transfer. As we are allowed to use 16-bytes in a burst. That will 
trigger less burst than BEATS_4 to transfer same amount of data.

We found this patch can fix ISI FIFO overflow error when system is in a 
situation that with a very high memory load.
If we only allow 4-bytes in a burst, that need more burst to transfer 
data and less effective.

> What happens then if the data amount isn't a multiple of the
> transfer size?

BEATS_16 means allowed 4-bytes, 8-bytes and 16-bytes in a burst. So it 
still can perform a 4-bytes transfer if the data is not multiple of 16.
I think all the data transfer are aligned with 4 bytes. The DMA address 
should be aligned with 4 bytes as well.

Best Regards,
Josh Wu
>
> Thanks
> Guennadi
>
>> Reported-by: Bo Shen <voice.shen@atmel.com>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>>   drivers/media/platform/soc_camera/atmel-isi.c | 2 ++
>>   include/media/atmel-isi.h                     | 4 ++++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>> index ee5650f..fda587b 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -839,6 +839,8 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
>>   	if (isi->pdata.full_mode)
>>   		cfg1 |= ISI_CFG1_FULL_MODE;
>>   
>> +	cfg1 |= ISI_CFG1_THMASK_BEATS_16;
>> +
>>   	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
>>   	isi_writel(isi, ISI_CFG1, cfg1);
>>   
>> diff --git a/include/media/atmel-isi.h b/include/media/atmel-isi.h
>> index c2e5703..6008b09 100644
>> --- a/include/media/atmel-isi.h
>> +++ b/include/media/atmel-isi.h
>> @@ -59,6 +59,10 @@
>>   #define		ISI_CFG1_FRATE_DIV_MASK		(7 << 8)
>>   #define ISI_CFG1_DISCR				(1 << 11)
>>   #define ISI_CFG1_FULL_MODE			(1 << 12)
>> +/* Definition for THMASK(ISI_V2) */
>> +#define		ISI_CFG1_THMASK_BEATS_4		(0 << 13)
>> +#define		ISI_CFG1_THMASK_BEATS_8		(1 << 13)
>> +#define		ISI_CFG1_THMASK_BEATS_16	(2 << 13)
>>   
>>   /* Bitfields in CFG2 */
>>   #define ISI_CFG2_GRAYSCALE			(1 << 13)
>> -- 
>> 1.9.1
>>

WARNING: multiple messages have this Message-ID (diff)
From: Josh Wu <josh.wu@atmel.com>
To: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: <linux-media@vger.kernel.org>, <m.chehab@samsung.com>,
	<linux-arm-kernel@lists.infradead.org>, <voice.shen@atmel.com>,
	<nicolas.ferre@atmel.com>
Subject: Re: [PATCH] media: atmel-isi: increase the burst length to improve the performance
Date: Wed, 26 Nov 2014 10:54:48 +0800	[thread overview]
Message-ID: <547540F8.7040901@atmel.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1411252319430.17362@axis700.grange>

Hi, Gunenadi

On 11/26/2014 6:21 AM, Guennadi Liakhovetski wrote:
> Hi Josh,
>
> On Tue, 25 Nov 2014, Josh Wu wrote:
>
>> The burst length could be BEATS_4/8/16. Before this patch, isi use default
>> value BEATS_4. To imporve the performance we could set it to BEATS_16.
>>
>> Otherwise sometime it would cause the ISI overflow error.
> Without looking at datasheets - what does this bit do? Change the transfer
> length?
Atmel ISI has two internal 32-bytes FIFOs, one for Preview, another for 
Codec.
This field is the threshold to trigger the FIFO transfer to AHB by DMA.
We can set the threshold to allow 4-bytes, 8-bytes or 16-bytes for a burst.
BEATS_4, means only allow 4-bytes in a burst.
BEATS_8, means only allow 4-bytes and 8-bytes in a burst.
BEATS_16, means allow 4-bytes, 8-bytes and 16-bytes in a burst.

So BEATS_16 will use the DMA more efficient if has lots of data to 
transfer. As we are allowed to use 16-bytes in a burst. That will 
trigger less burst than BEATS_4 to transfer same amount of data.

We found this patch can fix ISI FIFO overflow error when system is in a 
situation that with a very high memory load.
If we only allow 4-bytes in a burst, that need more burst to transfer 
data and less effective.

> What happens then if the data amount isn't a multiple of the
> transfer size?

BEATS_16 means allowed 4-bytes, 8-bytes and 16-bytes in a burst. So it 
still can perform a 4-bytes transfer if the data is not multiple of 16.
I think all the data transfer are aligned with 4 bytes. The DMA address 
should be aligned with 4 bytes as well.

Best Regards,
Josh Wu
>
> Thanks
> Guennadi
>
>> Reported-by: Bo Shen <voice.shen@atmel.com>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>>   drivers/media/platform/soc_camera/atmel-isi.c | 2 ++
>>   include/media/atmel-isi.h                     | 4 ++++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>> index ee5650f..fda587b 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -839,6 +839,8 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
>>   	if (isi->pdata.full_mode)
>>   		cfg1 |= ISI_CFG1_FULL_MODE;
>>   
>> +	cfg1 |= ISI_CFG1_THMASK_BEATS_16;
>> +
>>   	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
>>   	isi_writel(isi, ISI_CFG1, cfg1);
>>   
>> diff --git a/include/media/atmel-isi.h b/include/media/atmel-isi.h
>> index c2e5703..6008b09 100644
>> --- a/include/media/atmel-isi.h
>> +++ b/include/media/atmel-isi.h
>> @@ -59,6 +59,10 @@
>>   #define		ISI_CFG1_FRATE_DIV_MASK		(7 << 8)
>>   #define ISI_CFG1_DISCR				(1 << 11)
>>   #define ISI_CFG1_FULL_MODE			(1 << 12)
>> +/* Definition for THMASK(ISI_V2) */
>> +#define		ISI_CFG1_THMASK_BEATS_4		(0 << 13)
>> +#define		ISI_CFG1_THMASK_BEATS_8		(1 << 13)
>> +#define		ISI_CFG1_THMASK_BEATS_16	(2 << 13)
>>   
>>   /* Bitfields in CFG2 */
>>   #define ISI_CFG2_GRAYSCALE			(1 << 13)
>> -- 
>> 1.9.1
>>


  reply	other threads:[~2014-11-26  2:54 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-25  9:30 [PATCH] media: atmel-isi: increase the burst length to improve the performance Josh Wu
2014-11-25  9:30 ` Josh Wu
2014-11-25 22:21 ` Guennadi Liakhovetski
2014-11-25 22:21   ` Guennadi Liakhovetski
2014-11-26  2:54   ` Josh Wu [this message]
2014-11-26  2:54     ` Josh Wu
2015-02-02 10:15 ` Josh Wu
2015-02-02 10:15   ` Josh Wu
2015-02-02 10:22   ` Guennadi Liakhovetski
2015-02-02 10:22     ` Guennadi Liakhovetski
2015-02-03  5:50     ` Josh Wu
2015-02-03  5:50       ` Josh Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=547540F8.7040901@atmel.com \
    --to=josh.wu@atmel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.