LinuxTV Patchwork [1/3] media: atmel: atmel-isc: limit incoming pixels per frame

login
register
mail settings
Submitter Eugen Hristev
Date April 12, 2019, 10:19 a.m.
Message ID <1555064098-19310-2-git-send-email-eugen.hristev@microchip.com>
Download mbox | patch
Permalink /patch/55635/
State New
Delegated to: Hans Verkuil
Headers show

Comments

Eugen Hristev - April 12, 2019, 10:19 a.m.
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(+)

-- 
2.7.4
Hans Verkuil - April 12, 2019, 11:50 a.m.
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


> +		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);
>
Eugen Hristev - April 12, 2019, 12:02 p.m.
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

> 

>
Hans Verkuil - April 12, 2019, 12:06 p.m.
On 4/12/19 2:02 PM, Eugen.Hristev@microchip.com wrote:
> 
> 
> 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...

Ah, now I see. I'll park this patch series until the pull request containing
those two patches is merged. Feel free to remind me of this series once Mauro
merged that pull request.

Regards,

	Hans

> 
> 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
>>
>>
Eugen Hristev - April 12, 2019, 12:13 p.m.
On 12.04.2019 15:06, Hans Verkuil wrote:
> External E-Mail

> 

> 

> On 4/12/19 2:02 PM, Eugen.Hristev@microchip.com wrote:

>>

>>

>> 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...

> 

> Ah, now I see. I'll park this patch series until the pull request containing

> those two patches is merged. Feel free to remind me of this series once Mauro

> merged that pull request.


Thank you.

I mostly sent this series to get your early review. It would be 
important to get the new feature set integrated, so, thank you for that, 
and I will send the v2 on the new feature set, so I can work on it until 
the first patches are merged.

Eugen

> 

> Regards,

> 

> 	Hans

> 

>>

>> 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

>>>

>>>

>

Patch

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)) {

+		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);

Privacy Policy