On 12.04.2019 15:09, Eugen Hristev wrote:
>
>
> On 12.04.2019 15:06, Hans Verkuil wrote:
>> External E-Mail
>>
>>
>> On 4/12/19 2:02 PM, Eugen.Hristev@xxxxxxxxxxxxx wrote:
>>>
>>>
>>> On 12.04.2019 14:50, Hans Verkuil wrote:
>>>
>>>>
>>>> On 4/12/19 12:19 PM, Eugen.Hristev@xxxxxxxxxxxxx wrote:
>>>>> From: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
>>>>>
>>>>> 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@xxxxxxxxxxxxx>
>>>>> ---
>>>>> 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.
Hello Hans,
This series should apply OK now on latest media_tree.git
Eugen
>
> 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
>>>
>>>
>>>>
[snip]
