| Message ID | 1416907825-23826-1-git-send-email-josh.wu@atmel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? What happens then if the data amount isn't a multiple of the transfer size? 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 >
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 >>
Hi, Guennadi Ping? what about the status of this patch? Best Regards, Josh Wu On 11/25/2014 5:30 PM, 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. > > 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)
Hi Josh, On Mon, 2 Feb 2015, Josh Wu wrote: > Hi, Guennadi > > Ping? what about the status of this patch? Right, got lost, sorry... Added to the queue now. Thanks Guennadi > Best Regards, > Josh Wu > > On 11/25/2014 5:30 PM, 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. > > > > 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) >
Hi, Guennadi On 2/2/2015 6:22 PM, Guennadi Liakhovetski wrote: > Hi Josh, > > On Mon, 2 Feb 2015, Josh Wu wrote: > >> Hi, Guennadi >> >> Ping? what about the status of this patch? > Right, got lost, sorry... Added to the queue now. Thank you. Best Regards, Josh Wu > > Thanks > Guennadi > >> Best Regards, >> Josh Wu >> >> On 11/25/2014 5:30 PM, 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. >>> >>> 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)
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)
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. 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(+)