Re: [PATCH v1 5/5] [media] stm32-dcmi: g_/s_selection crop support

2017-08-21 Thread Hugues FRUCHET
Hi Hans, thanks for reviewing

On 08/04/2017 02:26 PM, Hans Verkuil wrote:
> On 28/07/17 12:05, Hugues Fruchet wrote:
>> Implements g_/s_selection crop support by using DCMI crop
>> hardware feature.
>> User can first get the maximum supported resolution of the sensor
>> by calling g_selection(V4L2_SEL_TGT_CROP_BOUNDS).
>> Then user call to s_selection(V4L2_SEL_TGT_CROP) will reset sensor
>> to its maximum resolution and crop request is saved for later usage
>> in s_fmt().
>> Next call to s_fmt() will check if sensor can do frame size request
>> with crop request. If sensor supports only discrete frame sizes,
>> the frame size which is larger than user request is selected in
>> order to be able to match the crop request. Then s_fmt() resolution
>> user request is adjusted to match crop request resolution.
>>
>> Signed-off-by: Hugues Fruchet 
>> ---
>>   drivers/media/platform/stm32/stm32-dcmi.c | 367 
>> +-
>>   1 file changed, 363 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index 2be56b6..f1fb0b3 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -33,6 +33,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   
>>   #define DRV_NAME "stm32-dcmi"
>> @@ -107,6 +108,11 @@ struct dcmi_format {
>>  u8  bpp;
>>   };
>>   
>> +struct dcmi_framesize {
>> +u32 width;
>> +u32 height;
>> +};
>> +
>>   struct dcmi_buf {
>>  struct vb2_v4l2_buffer  vb;
>>  boolprepared;
>> @@ -131,10 +137,16 @@ struct stm32_dcmi {
>>  struct v4l2_async_notifier  notifier;
>>  struct dcmi_graph_entityentity;
>>  struct v4l2_format  fmt;
>> +struct v4l2_rectcrop;
>> +booldo_crop;
>>   
>>  const struct dcmi_format**sd_formats;
>>  unsigned intnb_of_sd_formats;
>>  const struct dcmi_format*sd_format;
>> +struct dcmi_framesize   *sd_framesizes;
>> +unsigned intnb_of_sd_framesizes;
> 
> num_of_sd_framesizes is better.
> 

OK, will rename.

>> +struct dcmi_framesize   sd_framesize;
>> +struct v4l2_rectsd_bounds;
>>   
>>  /* Protect this data structure */
>>  struct mutexlock;
>> @@ -325,6 +337,28 @@ static int dcmi_start_capture(struct stm32_dcmi *dcmi)
>>  return 0;
>>   }
>>   
>> +static void dcmi_set_crop(struct stm32_dcmi *dcmi)
>> +{
>> +u32 size, start;
>> +
>> +/* Crop resolution */
>> +size = ((dcmi->crop.height - 1) << 16) |
>> +((dcmi->crop.width << 1) - 1);
>> +reg_write(dcmi->regs, DCMI_CWSIZE, size);
>> +
>> +/* Crop start point */
>> +start = ((dcmi->crop.top) << 16) |
>> + ((dcmi->crop.left << 1));
>> +reg_write(dcmi->regs, DCMI_CWSTRT, start);
>> +
>> +dev_dbg(dcmi->dev, "Cropping to %ux%u@%u:%u\n",
>> +dcmi->crop.width, dcmi->crop.height,
>> +dcmi->crop.left, dcmi->crop.top);
>> +
>> +/* Enable crop */
>> +reg_set(dcmi->regs, DCMI_CR, CR_CROP);
>> +}
>> +
>>   static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>>   {
>>  struct stm32_dcmi *dcmi = arg;
>> @@ -540,6 +574,10 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
>> unsigned int count)
>>   
>>  reg_write(dcmi->regs, DCMI_CR, val);
>>   
>> +/* Set crop */
>> +if (dcmi->do_crop)
>> +dcmi_set_crop(dcmi);
>> +
>>  /* Enable dcmi */
>>  reg_set(dcmi->regs, DCMI_CR, CR_ENABLE);
>>   
>> @@ -697,10 +735,37 @@ static const struct dcmi_format 
>> *find_format_by_fourcc(struct stm32_dcmi *dcmi,
>>  return NULL;
>>   }
>>   
>> +static void __find_outer_frame_size(struct stm32_dcmi *dcmi,
>> +struct v4l2_pix_format *pix,
>> +struct dcmi_framesize *framesize)
>> +{
>> +struct dcmi_framesize *match = NULL;
>> +unsigned int i;
>> +unsigned int min_err = UINT_MAX;
>> +
>> +for (i = 0; i < dcmi->nb_of_sd_framesizes; i++) {
>> +struct dcmi_framesize *fsize = >sd_framesizes[i];
>> +int w_err = (fsize->width - pix->width);
>> +int h_err = (fsize->height - pix->height);
>> +int err = w_err + h_err;
>> +
>> +if ((w_err >= 0) && (h_err >= 0) && (err < min_err)) {
>> +min_err = err;
>> +match = fsize;
>> +}
>> +}
>> +if (!match)
>> +match = >sd_framesizes[0];
>> +
>> +*framesize = *match;
>> +}
>> +
>>   static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
>> -const struct dcmi_format **sd_format)
>> +const struct dcmi_format **sd_format,
>> +   

Re: [PATCH v1 5/5] [media] stm32-dcmi: g_/s_selection crop support

2017-08-04 Thread Hans Verkuil
On 28/07/17 12:05, Hugues Fruchet wrote:
> Implements g_/s_selection crop support by using DCMI crop
> hardware feature.
> User can first get the maximum supported resolution of the sensor
> by calling g_selection(V4L2_SEL_TGT_CROP_BOUNDS).
> Then user call to s_selection(V4L2_SEL_TGT_CROP) will reset sensor
> to its maximum resolution and crop request is saved for later usage
> in s_fmt().
> Next call to s_fmt() will check if sensor can do frame size request
> with crop request. If sensor supports only discrete frame sizes,
> the frame size which is larger than user request is selected in
> order to be able to match the crop request. Then s_fmt() resolution
> user request is adjusted to match crop request resolution.
> 
> Signed-off-by: Hugues Fruchet 
> ---
>  drivers/media/platform/stm32/stm32-dcmi.c | 367 
> +-
>  1 file changed, 363 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index 2be56b6..f1fb0b3 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define DRV_NAME "stm32-dcmi"
> @@ -107,6 +108,11 @@ struct dcmi_format {
>   u8  bpp;
>  };
>  
> +struct dcmi_framesize {
> + u32 width;
> + u32 height;
> +};
> +
>  struct dcmi_buf {
>   struct vb2_v4l2_buffer  vb;
>   boolprepared;
> @@ -131,10 +137,16 @@ struct stm32_dcmi {
>   struct v4l2_async_notifier  notifier;
>   struct dcmi_graph_entityentity;
>   struct v4l2_format  fmt;
> + struct v4l2_rectcrop;
> + booldo_crop;
>  
>   const struct dcmi_format**sd_formats;
>   unsigned intnb_of_sd_formats;
>   const struct dcmi_format*sd_format;
> + struct dcmi_framesize   *sd_framesizes;
> + unsigned intnb_of_sd_framesizes;

num_of_sd_framesizes is better.

> + struct dcmi_framesize   sd_framesize;
> + struct v4l2_rectsd_bounds;
>  
>   /* Protect this data structure */
>   struct mutexlock;
> @@ -325,6 +337,28 @@ static int dcmi_start_capture(struct stm32_dcmi *dcmi)
>   return 0;
>  }
>  
> +static void dcmi_set_crop(struct stm32_dcmi *dcmi)
> +{
> + u32 size, start;
> +
> + /* Crop resolution */
> + size = ((dcmi->crop.height - 1) << 16) |
> + ((dcmi->crop.width << 1) - 1);
> + reg_write(dcmi->regs, DCMI_CWSIZE, size);
> +
> + /* Crop start point */
> + start = ((dcmi->crop.top) << 16) |
> +  ((dcmi->crop.left << 1));
> + reg_write(dcmi->regs, DCMI_CWSTRT, start);
> +
> + dev_dbg(dcmi->dev, "Cropping to %ux%u@%u:%u\n",
> + dcmi->crop.width, dcmi->crop.height,
> + dcmi->crop.left, dcmi->crop.top);
> +
> + /* Enable crop */
> + reg_set(dcmi->regs, DCMI_CR, CR_CROP);
> +}
> +
>  static irqreturn_t dcmi_irq_thread(int irq, void *arg)
>  {
>   struct stm32_dcmi *dcmi = arg;
> @@ -540,6 +574,10 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
> unsigned int count)
>  
>   reg_write(dcmi->regs, DCMI_CR, val);
>  
> + /* Set crop */
> + if (dcmi->do_crop)
> + dcmi_set_crop(dcmi);
> +
>   /* Enable dcmi */
>   reg_set(dcmi->regs, DCMI_CR, CR_ENABLE);
>  
> @@ -697,10 +735,37 @@ static const struct dcmi_format 
> *find_format_by_fourcc(struct stm32_dcmi *dcmi,
>   return NULL;
>  }
>  
> +static void __find_outer_frame_size(struct stm32_dcmi *dcmi,
> + struct v4l2_pix_format *pix,
> + struct dcmi_framesize *framesize)
> +{
> + struct dcmi_framesize *match = NULL;
> + unsigned int i;
> + unsigned int min_err = UINT_MAX;
> +
> + for (i = 0; i < dcmi->nb_of_sd_framesizes; i++) {
> + struct dcmi_framesize *fsize = >sd_framesizes[i];
> + int w_err = (fsize->width - pix->width);
> + int h_err = (fsize->height - pix->height);
> + int err = w_err + h_err;
> +
> + if ((w_err >= 0) && (h_err >= 0) && (err < min_err)) {
> + min_err = err;
> + match = fsize;
> + }
> + }
> + if (!match)
> + match = >sd_framesizes[0];
> +
> + *framesize = *match;
> +}
> +
>  static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
> - const struct dcmi_format **sd_format)
> + const struct dcmi_format **sd_format,
> + struct dcmi_framesize *sd_framesize)
>  {
>   const struct dcmi_format *sd_fmt;
> + struct dcmi_framesize sd_fsize;
>   struct v4l2_pix_format *pix = >fmt.pix;
>   struct 

Re: [PATCH v1 5/5] [media] stm32-dcmi: g_/s_selection crop support

2017-07-29 Thread kbuild test robot
Hi Hugues,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.13-rc2 next-20170728]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Hugues-Fruchet/STM32-DCMI-camera-interface-crop-support/20170730-114803
base:   git://linuxtv.org/media_tree.git master
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/media//platform/stm32/stm32-dcmi.c: In function 
'dcmi_graph_notify_complete':
>> drivers/media//platform/stm32/stm32-dcmi.c:1445:5: warning: 'ret' may be 
>> used uninitialized in this function [-Wmaybe-uninitialized]
 if (ret) {
^

vim +/ret +1445 drivers/media//platform/stm32/stm32-dcmi.c

  1431  
  1432  static int dcmi_graph_notify_complete(struct v4l2_async_notifier 
*notifier)
  1433  {
  1434  struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier);
  1435  int ret;
  1436  
  1437  dcmi->vdev->ctrl_handler = dcmi->entity.subdev->ctrl_handler;
  1438  ret = dcmi_formats_init(dcmi);
  1439  if (ret) {
  1440  dev_err(dcmi->dev, "No supported mediabus format 
found\n");
  1441  return ret;
  1442  }
  1443  
  1444  ret = dcmi_framesizes_init(dcmi);
> 1445  if (ret) {
  1446  dev_err(dcmi->dev, "Could not initialize framesizes\n");
  1447  return ret;
  1448  }
  1449  
  1450  ret = dcmi_get_sensor_bounds(dcmi, >sd_bounds);
  1451  if (ret) {
  1452  dev_err(dcmi->dev, "Could not get sensor bounds\n");
  1453  return ret;
  1454  }
  1455  
  1456  ret = dcmi_set_default_fmt(dcmi);
  1457  if (ret) {
  1458  dev_err(dcmi->dev, "Could not set default format\n");
  1459  return ret;
  1460  }
  1461  
  1462  ret = video_register_device(dcmi->vdev, VFL_TYPE_GRABBER, -1);
  1463  if (ret) {
  1464  dev_err(dcmi->dev, "Failed to register video device\n");
  1465  return ret;
  1466  }
  1467  
  1468  dev_dbg(dcmi->dev, "Device registered as %s\n",
  1469  video_device_node_name(dcmi->vdev));
  1470  return 0;
  1471  }
  1472  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip


[PATCH v1 5/5] [media] stm32-dcmi: g_/s_selection crop support

2017-07-28 Thread Hugues Fruchet
Implements g_/s_selection crop support by using DCMI crop
hardware feature.
User can first get the maximum supported resolution of the sensor
by calling g_selection(V4L2_SEL_TGT_CROP_BOUNDS).
Then user call to s_selection(V4L2_SEL_TGT_CROP) will reset sensor
to its maximum resolution and crop request is saved for later usage
in s_fmt().
Next call to s_fmt() will check if sensor can do frame size request
with crop request. If sensor supports only discrete frame sizes,
the frame size which is larger than user request is selected in
order to be able to match the crop request. Then s_fmt() resolution
user request is adjusted to match crop request resolution.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/platform/stm32/stm32-dcmi.c | 367 +-
 1 file changed, 363 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 2be56b6..f1fb0b3 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define DRV_NAME "stm32-dcmi"
@@ -107,6 +108,11 @@ struct dcmi_format {
u8  bpp;
 };
 
+struct dcmi_framesize {
+   u32 width;
+   u32 height;
+};
+
 struct dcmi_buf {
struct vb2_v4l2_buffer  vb;
boolprepared;
@@ -131,10 +137,16 @@ struct stm32_dcmi {
struct v4l2_async_notifier  notifier;
struct dcmi_graph_entityentity;
struct v4l2_format  fmt;
+   struct v4l2_rectcrop;
+   booldo_crop;
 
const struct dcmi_format**sd_formats;
unsigned intnb_of_sd_formats;
const struct dcmi_format*sd_format;
+   struct dcmi_framesize   *sd_framesizes;
+   unsigned intnb_of_sd_framesizes;
+   struct dcmi_framesize   sd_framesize;
+   struct v4l2_rectsd_bounds;
 
/* Protect this data structure */
struct mutexlock;
@@ -325,6 +337,28 @@ static int dcmi_start_capture(struct stm32_dcmi *dcmi)
return 0;
 }
 
+static void dcmi_set_crop(struct stm32_dcmi *dcmi)
+{
+   u32 size, start;
+
+   /* Crop resolution */
+   size = ((dcmi->crop.height - 1) << 16) |
+   ((dcmi->crop.width << 1) - 1);
+   reg_write(dcmi->regs, DCMI_CWSIZE, size);
+
+   /* Crop start point */
+   start = ((dcmi->crop.top) << 16) |
+((dcmi->crop.left << 1));
+   reg_write(dcmi->regs, DCMI_CWSTRT, start);
+
+   dev_dbg(dcmi->dev, "Cropping to %ux%u@%u:%u\n",
+   dcmi->crop.width, dcmi->crop.height,
+   dcmi->crop.left, dcmi->crop.top);
+
+   /* Enable crop */
+   reg_set(dcmi->regs, DCMI_CR, CR_CROP);
+}
+
 static irqreturn_t dcmi_irq_thread(int irq, void *arg)
 {
struct stm32_dcmi *dcmi = arg;
@@ -540,6 +574,10 @@ static int dcmi_start_streaming(struct vb2_queue *vq, 
unsigned int count)
 
reg_write(dcmi->regs, DCMI_CR, val);
 
+   /* Set crop */
+   if (dcmi->do_crop)
+   dcmi_set_crop(dcmi);
+
/* Enable dcmi */
reg_set(dcmi->regs, DCMI_CR, CR_ENABLE);
 
@@ -697,10 +735,37 @@ static const struct dcmi_format 
*find_format_by_fourcc(struct stm32_dcmi *dcmi,
return NULL;
 }
 
+static void __find_outer_frame_size(struct stm32_dcmi *dcmi,
+   struct v4l2_pix_format *pix,
+   struct dcmi_framesize *framesize)
+{
+   struct dcmi_framesize *match = NULL;
+   unsigned int i;
+   unsigned int min_err = UINT_MAX;
+
+   for (i = 0; i < dcmi->nb_of_sd_framesizes; i++) {
+   struct dcmi_framesize *fsize = >sd_framesizes[i];
+   int w_err = (fsize->width - pix->width);
+   int h_err = (fsize->height - pix->height);
+   int err = w_err + h_err;
+
+   if ((w_err >= 0) && (h_err >= 0) && (err < min_err)) {
+   min_err = err;
+   match = fsize;
+   }
+   }
+   if (!match)
+   match = >sd_framesizes[0];
+
+   *framesize = *match;
+}
+
 static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
-   const struct dcmi_format **sd_format)
+   const struct dcmi_format **sd_format,
+   struct dcmi_framesize *sd_framesize)
 {
const struct dcmi_format *sd_fmt;
+   struct dcmi_framesize sd_fsize;
struct v4l2_pix_format *pix = >fmt.pix;
struct v4l2_subdev_pad_config pad_cfg;
struct v4l2_subdev_format format = {
@@ -718,6 +783,17 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct 
v4l2_format *f,
pix->width = clamp(pix->width, MIN_WIDTH,