lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f6e42811-7b72-ce12-98b5-2f48fe5fb8d0@microchip.com>
Date:   Mon, 12 Apr 2021 10:04:14 +0000
From:   <Eugen.Hristev@...rochip.com>
To:     <jacopo@...ndi.org>
CC:     <devicetree@...r.kernel.org>, <linux-media@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 04/30] media: atmel: atmel-isc: specialize max width
 and max height

On 4/12/21 12:53 PM, Jacopo Mondi wrote:
> Ups,
> 
> On Mon, Apr 12, 2021 at 11:43:12AM +0200, Jacopo Mondi wrote:
>> Hi Eugene,
>>
>> On Mon, Apr 05, 2021 at 06:50:39PM +0300, Eugen Hristev wrote:
>>> Move the max width and max height constants to the product specific driver
>>> and have them in the device struct.
>>>
>>> Signed-off-by: Eugen Hristev <eugen.hristev@...rochip.com>
>>> ---
>>>   drivers/media/platform/atmel/atmel-isc-base.c | 28 +++++++++----------
>>>   drivers/media/platform/atmel/atmel-isc.h      |  9 ++++--
>>>   .../media/platform/atmel/atmel-sama5d2-isc.c  |  7 +++--
>>>   3 files changed, 25 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
>>> index 45fc8dbb7943..350076dd029a 100644
>>> --- a/drivers/media/platform/atmel/atmel-isc-base.c
>>> +++ b/drivers/media/platform/atmel/atmel-isc-base.c
>>> @@ -1204,8 +1204,8 @@ static void isc_try_fse(struct isc_device *isc,
>>>       * just use the maximum ISC can receive.
>>>       */
>>>      if (ret) {
>>> -           pad_cfg->try_crop.width = ISC_MAX_SUPPORT_WIDTH;
>>> -           pad_cfg->try_crop.height = ISC_MAX_SUPPORT_HEIGHT;
>>> +           pad_cfg->try_crop.width = isc->max_width;
>>> +           pad_cfg->try_crop.height = isc->max_height;
>>>      } else {
>>>              pad_cfg->try_crop.width = fse.max_width;
>>>              pad_cfg->try_crop.height = fse.max_height;
>>> @@ -1282,10 +1282,10 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
>>>      isc->try_config.sd_format = sd_fmt;
>>>
>>>      /* Limit to Atmel ISC hardware capabilities */
>>> -   if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)
>>> -           pixfmt->width = ISC_MAX_SUPPORT_WIDTH;
>>> -   if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)
>>> -           pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;
>>> +   if (pixfmt->width > isc->max_width)
>>> +           pixfmt->width = isc->max_width;
>>> +   if (pixfmt->height > isc->max_height)
>>> +           pixfmt->height = isc->max_height;
>>>
>>>      /*
>>>       * The mbus format is the one the subdev outputs.
>>> @@ -1327,10 +1327,10 @@ static int isc_try_fmt(struct isc_device *isc, struct v4l2_format *f,
>>>      v4l2_fill_pix_format(pixfmt, &format.format);
>>>
>>>      /* Limit to Atmel ISC hardware capabilities */
>>> -   if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)
>>> -           pixfmt->width = ISC_MAX_SUPPORT_WIDTH;
>>> -   if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)
>>> -           pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;
>>> +   if (pixfmt->width > isc->max_width)
>>> +           pixfmt->width = isc->max_width;
>>> +   if (pixfmt->height > isc->max_height)
>>> +           pixfmt->height = isc->max_height;
>>
>> What happens if the sensor sends a frame larger that the ISC max
>> supported sizes ?
>>
> 
> I meant to ask this question on the previous patch :/

Hi Jacopo,

The ISC has a feature in the PFE module (parallel front end), the first 
pixel capturing module, which will automatically crop the frame at the 
maximum size (or configured frame size).

here is the commit that implements this safety mechanism :

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=253ccf34232ae3b47497e5e55aef3ac48821425c

Eugen

> 
>>>
>>>      pixfmt->field = V4L2_FIELD_NONE;
>>>      pixfmt->bytesperline = (pixfmt->width * isc->try_config.bpp) >> 3;
>>> @@ -1368,10 +1368,10 @@ static int isc_set_fmt(struct isc_device *isc, struct v4l2_format *f)
>>>              return ret;
>>>
>>>      /* Limit to Atmel ISC hardware capabilities */
>>> -   if (pixfmt->width > ISC_MAX_SUPPORT_WIDTH)
>>> -           pixfmt->width = ISC_MAX_SUPPORT_WIDTH;
>>> -   if (pixfmt->height > ISC_MAX_SUPPORT_HEIGHT)
>>> -           pixfmt->height = ISC_MAX_SUPPORT_HEIGHT;
>>> +   if (f->fmt.pix.width > isc->max_width)
>>> +           f->fmt.pix.width = isc->max_width;
>>> +   if (f->fmt.pix.height > isc->max_height)
>>> +           f->fmt.pix.height = isc->max_height;
>>>
>>>      isc->fmt = *f;
>>>
>>> diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
>>> index 8d81d9967ad2..6becc6c3aaf0 100644
>>> --- a/drivers/media/platform/atmel/atmel-isc.h
>>> +++ b/drivers/media/platform/atmel/atmel-isc.h
>>> @@ -10,9 +10,6 @@
>>>    */
>>>   #ifndef _ATMEL_ISC_H_
>>>
>>> -#define ISC_MAX_SUPPORT_WIDTH   2592
>>> -#define ISC_MAX_SUPPORT_HEIGHT  1944
>>> -
>>>   #define ISC_CLK_MAX_DIV            255
>>>
>>>   enum isc_clk_id {
>>> @@ -191,6 +188,9 @@ struct isc_ctrls {
>>>    * @gamma_table:   pointer to the table with gamma values, has
>>>    *                 gamma_max sets of GAMMA_ENTRIES entries each
>>>    * @gamma_max:             maximum number of sets of inside the gamma_table
>>> + *
>>> + * @max_width:             maximum frame width, dependent on the internal RAM
>>> + * @max_height:            maximum frame height, dependent on the internal RAM
>>>    */
>>>   struct isc_device {
>>>      struct regmap           *regmap;
>>> @@ -254,6 +254,9 @@ struct isc_device {
>>>      /* pointer to the defined gamma table */
>>>      const u32       (*gamma_table)[GAMMA_ENTRIES];
>>>      u32             gamma_max;
>>> +
>>> +   u32             max_width;
>>> +   u32             max_height;
>>>   };
>>>
>>>   extern struct isc_format formats_list[];
>>> diff --git a/drivers/media/platform/atmel/atmel-sama5d2-isc.c b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
>>> index f45d8b96bfb8..f8d1c8ba99b3 100644
>>> --- a/drivers/media/platform/atmel/atmel-sama5d2-isc.c
>>> +++ b/drivers/media/platform/atmel/atmel-sama5d2-isc.c
>>> @@ -49,8 +49,8 @@
>>>   #include "atmel-isc-regs.h"
>>>   #include "atmel-isc.h"
>>>
>>> -#define ISC_MAX_SUPPORT_WIDTH   2592
>>> -#define ISC_MAX_SUPPORT_HEIGHT  1944
>>> +#define ISC_SAMA5D2_MAX_SUPPORT_WIDTH   2592
>>> +#define ISC_SAMA5D2_MAX_SUPPORT_HEIGHT  1944
>>>
>>>   #define ISC_CLK_MAX_DIV            255
>>>
>>> @@ -195,6 +195,9 @@ static int atmel_isc_probe(struct platform_device *pdev)
>>>      isc->gamma_table = isc_sama5d2_gamma_table;
>>>      isc->gamma_max = 2;
>>>
>>> +   isc->max_width = ISC_SAMA5D2_MAX_SUPPORT_WIDTH;
>>> +   isc->max_height = ISC_SAMA5D2_MAX_SUPPORT_HEIGHT;
>>> +
>>>      ret = isc_pipeline_init(isc);
>>>      if (ret)
>>>              return ret;
>>> --
>>> 2.25.1
>>>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ