lkml.org 
[lkml]   [2019]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 1/3] media: atmel: atmel-isc: limit incoming pixels per frame
Date


On 12.04.2019 14:50, Hans Verkuil wrote:

>
> On 4/12/19 12:19 PM, Eugen.Hristev@microchip.com wrote:
>> From: Eugen Hristev <eugen.hristev@microchip.com>
>>
>> This will limit the incoming pixels per frame from the sensor.
>> Currently, the ISC will stop sampling the frame only when the vsync/hsync
>> are detected.
>> If we misconfigure the resolution in the sensor w.r.t. resolution in the ISC,
>> the buffer used for DMA in the ISC will be smaller than the number of pixels
>> that the ISC DMA engine will copy.
>> In this case it happens that the DMA will overwrite parts of the memory which
>> should not be written, leading to memory corruption.
>> To avoid this situation, use the PFE CFG1 and PFE CFG2 registers, which crop
>> the incoming frame to the resolution that we configure.
>> This way the DMA engine will never write more data than we expect it to.
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
>> drivers/media/platform/atmel/atmel-isc-regs.h | 19 +++++++++++++++
>> drivers/media/platform/atmel/atmel-isc.c | 34 +++++++++++++++++++++++++++
>> 2 files changed, 53 insertions(+)
>>
>> diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h b/drivers/media/platform/atmel/atmel-isc-regs.h
>> index 2aadc19..768a5ad 100644
>> --- a/drivers/media/platform/atmel/atmel-isc-regs.h
>> +++ b/drivers/media/platform/atmel/atmel-isc-regs.h
>> @@ -35,6 +35,25 @@
>> #define ISC_PFG_CFG0_BPS_TWELVE (0x0 << 28)
>> #define ISC_PFE_CFG0_BPS_MASK GENMASK(30, 28)
>>
>> +#define ISC_PFE_CFG0_COLEN BIT(12)
>> +#define ISC_PFE_CFG0_ROWEN BIT(13)
>> +
>> +/* ISC Parallel Front End Configuration 1 Register */
>> +#define ISC_PFE_CFG1 0x00000010
>> +
>> +#define ISC_PFE_CFG1_COLMIN(v) ((v))
>> +#define ISC_PFE_CFG1_COLMIN_MASK GENMASK(15, 0)
>> +#define ISC_PFE_CFG1_COLMAX(v) ((v) << 16)
>> +#define ISC_PFE_CFG1_COLMAX_MASK GENMASK(31, 16)
>> +
>> +/* ISC Parallel Front End Configuration 2 Register */
>> +#define ISC_PFE_CFG2 0x00000014
>> +
>> +#define ISC_PFE_CFG2_ROWMIN(v) ((v))
>> +#define ISC_PFE_CFG2_ROWMIN_MASK GENMASK(15, 0)
>> +#define ISC_PFE_CFG2_ROWMAX(v) ((v) << 16)
>> +#define ISC_PFE_CFG2_ROWMAX_MASK GENMASK(31, 16)
>> +
>> /* ISC Clock Enable Register */
>> #define ISC_CLKEN 0x00000018
>>
>> diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
>> index a10db16..ea7520a 100644
>> --- a/drivers/media/platform/atmel/atmel-isc.c
>> +++ b/drivers/media/platform/atmel/atmel-isc.c
>> @@ -721,6 +721,40 @@ static void isc_start_dma(struct isc_device *isc)
>> u32 sizeimage = isc->fmt.fmt.pix.sizeimage;
>> u32 dctrl_dview;
>> dma_addr_t addr0;
>> + u32 h, w;
>> +
>> + h = isc->fmt.fmt.pix.height;
>> + w = isc->fmt.fmt.pix.width;
>> +
>> + /*
>> + * In case the sensor is not RAW, it will output a pixel (12-16 bits)
>> + * with two samples on the ISC Data bus (which is 8-12)
>> + * ISC will count each sample, so, we need to multiply these values
>> + * by two, to get the real number of samples for the required pixels.
>> + */
>> + if (!ISC_IS_FORMAT_RAW(isc->config.sd_format->mbus_code)) {
>
> The ISC_IS_FORMAT_RAW define doesn't exist?!
>
> Something clearly went wrong...
>
> Regards,
>
> Hans

Hello Hans,

Sorry , I forgot to copy this from the previous series
(https://www.spinics.net/lists/linux-media/msg149501.html for reference):

It applies only on top of my previous patchset:
media: atmel: atmel-isc: removed ARGB32 added ABGR32 and XBGR32
media: atmel: atmel-isc: reworked driver and formats
available at:
https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=for-v5.2b&id=03ef1b56cba6ad17f6ead13c85a81e0e80fbc9d1

So it should work on top of those patches...

Eugen


>
>
>> + h <<= 1;
>> + w <<= 1;
>> + }
>> +
>> + /*
>> + * We limit the column/row count that the ISC will output according
>> + * to the configured resolution that we want.
>> + * This will avoid the situation where the sensor is misconfigured,
>> + * sending more data, and the ISC will just take it and DMA to memory,
>> + * causing corruption.
>> + */
>> + regmap_write(regmap, ISC_PFE_CFG1,
>> + (ISC_PFE_CFG1_COLMIN(0) & ISC_PFE_CFG1_COLMIN_MASK) |
>> + (ISC_PFE_CFG1_COLMAX(w - 1) & ISC_PFE_CFG1_COLMAX_MASK));
>> +
>> + regmap_write(regmap, ISC_PFE_CFG2,
>> + (ISC_PFE_CFG2_ROWMIN(0) & ISC_PFE_CFG2_ROWMIN_MASK) |
>> + (ISC_PFE_CFG2_ROWMAX(h - 1) & ISC_PFE_CFG2_ROWMAX_MASK));
>> +
>> + regmap_update_bits(regmap, ISC_PFE_CFG0,
>> + ISC_PFE_CFG0_COLEN | ISC_PFE_CFG0_ROWEN,
>> + ISC_PFE_CFG0_COLEN | ISC_PFE_CFG0_ROWEN);
>>
>> addr0 = vb2_dma_contig_plane_dma_addr(&isc->cur_frm->vb.vb2_buf, 0);
>> regmap_write(regmap, ISC_DAD0, addr0);
>>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
\
 
 \ /
  Last update: 2019-04-12 14:03    [W:0.064 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site