subject:"\[PATCHv5 07\/16\] atmel\-isi\: remove dependency of the soc\-camera framework"

Re: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

2017-03-13 Thread Nicolas Ferre

Le 13/03/2017 à 12:43, Hans Verkuil a écrit :
> On 03/12/2017 05:44 PM, Guennadi Liakhovetski wrote:
>> Hi Hans,
>>
>> Thanks for the patch. Why hasn't this patch been CCed to Josh Wu? Is he 
>> still at Atmel? Adding to CC to check.
> 
> To the best of my knowledge Josh no longer works for Atmel/Microchip, and 
> Songjun
> took over.

Yes absolutely, Josh Wu is no longer with us and Songjun and Ludovic
took over the maintenance.
But we have full confidence in experts like you guys and we thank you so
much Hans for having handled this move for atmel-isi.

>> A general comment: this patch doesn't only remove soc-camera dependencies, 
>> it also changes the code and the behaviour. I would prefer to break that 
>> down in multiple patches, or remove this driver completely and add a new 
>> one. I'll provide some comments, but of course I cannot make an extensive 
>> review of a 1200-line of change patch without knowing the hardware and the 
>> set ups well enough.
>>
>> On Sat, 11 Mar 2017, Hans Verkuil wrote:
>>
>>> From: Hans Verkuil 
>>>
>>> This patch converts the atmel-isi driver from a soc-camera driver to a 
>>> driver
>>> that is stand-alone.
>>>
>>> Signed-off-by: Hans Verkuil 
>>> ---
>>>  drivers/media/platform/soc_camera/Kconfig |3 +-
>>>  drivers/media/platform/soc_camera/atmel-isi.c | 1209 
>>> +++--
>>>  2 files changed, 714 insertions(+), 498 deletions(-)

[..]

>>> +static struct isi_format isi_formats[] = {
>>
>> This isn't a const array, you're modifying it during initialisation. Are 
>> we sure there aren't going to be any SoCs with more than one ISI?
> 
> When that happens, this should be changed at that time. I think it is unlikely
> since as I understand it ISI has been replaced by ISC (atmel-isc).

Yes, ISC has replaced ISI for all Atmel/Microchip MPUs onwards. We may
have several of them, but very likely never more than one ISI.

Regards,
-- 
Nicolas Ferre

Re: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

2017-03-13 Thread Hans Verkuil

On 03/12/2017 05:44 PM, Guennadi Liakhovetski wrote:
> Hi Hans,
> 
> Thanks for the patch. Why hasn't this patch been CCed to Josh Wu? Is he 
> still at Atmel? Adding to CC to check.

To the best of my knowledge Josh no longer works for Atmel/Microchip, and 
Songjun
took over.

> 
> A general comment: this patch doesn't only remove soc-camera dependencies, 
> it also changes the code and the behaviour. I would prefer to break that 
> down in multiple patches, or remove this driver completely and add a new 
> one. I'll provide some comments, but of course I cannot make an extensive 
> review of a 1200-line of change patch without knowing the hardware and the 
> set ups well enough.
> 
> On Sat, 11 Mar 2017, Hans Verkuil wrote:
> 
>> From: Hans Verkuil 
>>
>> This patch converts the atmel-isi driver from a soc-camera driver to a driver
>> that is stand-alone.
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  drivers/media/platform/soc_camera/Kconfig |3 +-
>>  drivers/media/platform/soc_camera/atmel-isi.c | 1209 
>> +++--
>>  2 files changed, 714 insertions(+), 498 deletions(-)
>>
>> diff --git a/drivers/media/platform/soc_camera/Kconfig 
>> b/drivers/media/platform/soc_camera/Kconfig
>> index 86d74788544f..a37ec91b026e 100644
>> --- a/drivers/media/platform/soc_camera/Kconfig
>> +++ b/drivers/media/platform/soc_camera/Kconfig
>> @@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
>>  
>>  config VIDEO_ATMEL_ISI
>>  tristate "ATMEL Image Sensor Interface (ISI) support"
>> -depends on VIDEO_DEV && SOC_CAMERA
>> +depends on VIDEO_V4L2 && OF && HAS_DMA
>>  depends on ARCH_AT91 || COMPILE_TEST
>> -depends on HAS_DMA
>>  select VIDEOBUF2_DMA_CONTIG
>>  ---help---
>>This module makes the ATMEL Image Sensor Interface available
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
>> b/drivers/media/platform/soc_camera/atmel-isi.c
>> index 46de657c3e6d..a6d60c2e207d 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -22,18 +22,22 @@
>>  #include 
>>  #include 
>>  #include 
>> -
>> -#include 
>> -#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  #include "atmel-isi.h"
>>  
>> -#define MAX_BUFFER_NUM  32
>>  #define MAX_SUPPORT_WIDTH   2048
>>  #define MAX_SUPPORT_HEIGHT  2048
>> -#define VID_LIMIT_BYTES (16 * 1024 * 1024)
>>  #define MIN_FRAME_RATE  15
>>  #define FRAME_INTERVAL_MILLI_SEC(1000 / MIN_FRAME_RATE)
>>  
>> @@ -65,9 +69,37 @@ struct frame_buffer {
>>  struct list_head list;
>>  };
>>  
>> +struct isi_graph_entity {
>> +struct device_node *node;
>> +
>> +struct v4l2_async_subdev asd;
>> +struct v4l2_subdev *subdev;
>> +};
>> +
>> +/*
>> + * struct isi_format - ISI media bus format information
>> + * @fourcc: Fourcc code for this format
>> + * @mbus_code:  V4L2 media bus format code.
>> + * @bpp:Bytes per pixel (when stored in memory)
>> + * @swap:   Byte swap configuration value
>> + * @support:Indicates format supported by subdev
>> + * @skip:   Skip duplicate format supported by subdev
>> + */
>> +struct isi_format {
>> +u32 fourcc;
>> +u32 mbus_code;
>> +u8  bpp;
>> +u32 swap;
>> +
>> +boolsupport;
>> +boolskip;
>> +};
>> +
>> +
>>  struct atmel_isi {
>>  /* Protects the access of variables shared with the ISR */
>> -spinlock_t  lock;
>> +spinlock_t  irqlock;
>> +struct device   *dev;
>>  void __iomem*regs;
>>  
>>  int sequence;
>> @@ -76,7 +108,7 @@ struct atmel_isi {
>>  struct fbd  *p_fb_descriptors;
>>  dma_addr_t  fb_descriptors_phys;
>>  struct  list_head dma_desc_head;
>> -struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
>> +struct isi_dma_desc dma_desc[VIDEO_MAX_FRAME];
>>  boolenable_preview_path;
>>  
>>  struct completion   complete;
>> @@ -90,9 +122,22 @@ struct atmel_isi {
>>  struct list_headvideo_buffer_list;
>>  struct frame_buffer *active;
>>  
>> -struct soc_camera_host  soc_host;
>> +struct v4l2_device  v4l2_dev;
>> +struct video_device *vdev;
>> +struct v4l2_async_notifier  notifier;
>> +struct isi_graph_entity entity;
>> +struct v4l2_format  fmt;
>> +
>> +struct isi_format   **user_formats;
>> +unsigned intnum_user_formats;
>> +const struct isi_format

Re: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

2017-03-12 Thread Guennadi Liakhovetski

Hi Hans,

Thanks for the patch. Why hasn't this patch been CCed to Josh Wu? Is he 
still at Atmel? Adding to CC to check.

A general comment: this patch doesn't only remove soc-camera dependencies, 
it also changes the code and the behaviour. I would prefer to break that 
down in multiple patches, or remove this driver completely and add a new 
one. I'll provide some comments, but of course I cannot make an extensive 
review of a 1200-line of change patch without knowing the hardware and the 
set ups well enough.

On Sat, 11 Mar 2017, Hans Verkuil wrote:

> From: Hans Verkuil 
> 
> This patch converts the atmel-isi driver from a soc-camera driver to a driver
> that is stand-alone.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/platform/soc_camera/Kconfig |3 +-
>  drivers/media/platform/soc_camera/atmel-isi.c | 1209 
> +++--
>  2 files changed, 714 insertions(+), 498 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/Kconfig 
> b/drivers/media/platform/soc_camera/Kconfig
> index 86d74788544f..a37ec91b026e 100644
> --- a/drivers/media/platform/soc_camera/Kconfig
> +++ b/drivers/media/platform/soc_camera/Kconfig
> @@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
>  
>  config VIDEO_ATMEL_ISI
>   tristate "ATMEL Image Sensor Interface (ISI) support"
> - depends on VIDEO_DEV && SOC_CAMERA
> + depends on VIDEO_V4L2 && OF && HAS_DMA
>   depends on ARCH_AT91 || COMPILE_TEST
> - depends on HAS_DMA
>   select VIDEOBUF2_DMA_CONTIG
>   ---help---
> This module makes the ATMEL Image Sensor Interface available
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
> b/drivers/media/platform/soc_camera/atmel-isi.c
> index 46de657c3e6d..a6d60c2e207d 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -22,18 +22,22 @@
>  #include 
>  #include 
>  #include 
> -
> -#include 
> -#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
>  #include 
>  #include 
> +#include 
>  
>  #include "atmel-isi.h"
>  
> -#define MAX_BUFFER_NUM   32
>  #define MAX_SUPPORT_WIDTH2048
>  #define MAX_SUPPORT_HEIGHT   2048
> -#define VID_LIMIT_BYTES  (16 * 1024 * 1024)
>  #define MIN_FRAME_RATE   15
>  #define FRAME_INTERVAL_MILLI_SEC (1000 / MIN_FRAME_RATE)
>  
> @@ -65,9 +69,37 @@ struct frame_buffer {
>   struct list_head list;
>  };
>  
> +struct isi_graph_entity {
> + struct device_node *node;
> +
> + struct v4l2_async_subdev asd;
> + struct v4l2_subdev *subdev;
> +};
> +
> +/*
> + * struct isi_format - ISI media bus format information
> + * @fourcc:  Fourcc code for this format
> + * @mbus_code:   V4L2 media bus format code.
> + * @bpp: Bytes per pixel (when stored in memory)
> + * @swap:Byte swap configuration value
> + * @support: Indicates format supported by subdev
> + * @skip:Skip duplicate format supported by subdev
> + */
> +struct isi_format {
> + u32 fourcc;
> + u32 mbus_code;
> + u8  bpp;
> + u32 swap;
> +
> + boolsupport;
> + boolskip;
> +};
> +
> +
>  struct atmel_isi {
>   /* Protects the access of variables shared with the ISR */
> - spinlock_t  lock;
> + spinlock_t  irqlock;
> + struct device   *dev;
>   void __iomem*regs;
>  
>   int sequence;
> @@ -76,7 +108,7 @@ struct atmel_isi {
>   struct fbd  *p_fb_descriptors;
>   dma_addr_t  fb_descriptors_phys;
>   struct  list_head dma_desc_head;
> - struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
> + struct isi_dma_desc dma_desc[VIDEO_MAX_FRAME];
>   boolenable_preview_path;
>  
>   struct completion   complete;
> @@ -90,9 +122,22 @@ struct atmel_isi {
>   struct list_headvideo_buffer_list;
>   struct frame_buffer *active;
>  
> - struct soc_camera_host  soc_host;
> + struct v4l2_device  v4l2_dev;
> + struct video_device *vdev;
> + struct v4l2_async_notifier  notifier;
> + struct isi_graph_entity entity;
> + struct v4l2_format  fmt;
> +
> + struct isi_format   **user_formats;
> + unsigned intnum_user_formats;
> + const struct isi_format *current_fmt;
> +
> + struct mutexlock;
> + struct vb2_queuequeue;
>  };
>  
> +#define notifier_to_isi(n) container_of(n, struct atmel_isi, notifier)
> +
>  static void isi_writel(struct atmel_isi

Re: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

2017-03-11 Thread Sakari Ailus

Hi Hans,

On Sat, Mar 11, 2017 at 12:23:19PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> This patch converts the atmel-isi driver from a soc-camera driver to a driver
> that is stand-alone.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/platform/soc_camera/Kconfig |3 +-
>  drivers/media/platform/soc_camera/atmel-isi.c | 1209 
> +++--
>  2 files changed, 714 insertions(+), 498 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/Kconfig 
> b/drivers/media/platform/soc_camera/Kconfig
> index 86d74788544f..a37ec91b026e 100644
> --- a/drivers/media/platform/soc_camera/Kconfig
> +++ b/drivers/media/platform/soc_camera/Kconfig
> @@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
>  
>  config VIDEO_ATMEL_ISI
>   tristate "ATMEL Image Sensor Interface (ISI) support"
> - depends on VIDEO_DEV && SOC_CAMERA
> + depends on VIDEO_V4L2 && OF && HAS_DMA
>   depends on ARCH_AT91 || COMPILE_TEST
> - depends on HAS_DMA
>   select VIDEOBUF2_DMA_CONTIG
>   ---help---
> This module makes the ATMEL Image Sensor Interface available
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
> b/drivers/media/platform/soc_camera/atmel-isi.c
> index 46de657c3e6d..a6d60c2e207d 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c

...

> +static int isi_graph_init(struct atmel_isi *isi)
> +{
> + struct v4l2_async_subdev **subdevs = NULL;
> + int ret;
> +
> + /* Parse the graph to extract a list of subdevice DT nodes. */
> + ret = isi_graph_parse(isi, isi->dev->of_node);
> + if (ret < 0) {
> + dev_err(isi->dev, "Graph parsing failed\n");
> + goto done;
> + }
> +
> + if (!ret) {
> + dev_err(isi->dev, "No subdev found in graph\n");
> + goto done;
> + }
> +
> + /* Register the subdevices notifier. */
> + subdevs = devm_kzalloc(isi->dev, sizeof(*subdevs), GFP_KERNEL);
> + if (subdevs == NULL) {
> + ret = -ENOMEM;
> + goto done;
> + }
> +
> + subdevs[0] = >entity.asd;
> +
> + isi->notifier.subdevs = subdevs;
> + isi->notifier.num_subdevs = 1;
> + isi->notifier.bound = isi_graph_notify_bound;
> + isi->notifier.unbind = isi_graph_notify_unbind;
> + isi->notifier.complete = isi_graph_notify_complete;
> +
> + ret = v4l2_async_notifier_register(>v4l2_dev, >notifier);
> + if (ret < 0) {
> + dev_err(isi->dev, "Notifier registration failed\n");
> + goto done;
> + }
> +
> + ret = 0;

You can replace this by

return 0;

And remove the if () below.

> +
> +done:
> + if (ret < 0) {
> + v4l2_async_notifier_unregister(>notifier);
> + of_node_put(isi->entity.node);
> + }
> +
> + return ret;
> +}
> +

Acked-by: Sakari Ailus 

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk

[PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

2017-03-11 Thread Hans Verkuil

From: Hans Verkuil 

This patch converts the atmel-isi driver from a soc-camera driver to a driver
that is stand-alone.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/soc_camera/Kconfig |3 +-
 drivers/media/platform/soc_camera/atmel-isi.c | 1209 +++--
 2 files changed, 714 insertions(+), 498 deletions(-)

diff --git a/drivers/media/platform/soc_camera/Kconfig 
b/drivers/media/platform/soc_camera/Kconfig
index 86d74788544f..a37ec91b026e 100644
--- a/drivers/media/platform/soc_camera/Kconfig
+++ b/drivers/media/platform/soc_camera/Kconfig
@@ -29,9 +29,8 @@ config VIDEO_SH_MOBILE_CEU
 
 config VIDEO_ATMEL_ISI
tristate "ATMEL Image Sensor Interface (ISI) support"
-   depends on VIDEO_DEV && SOC_CAMERA
+   depends on VIDEO_V4L2 && OF && HAS_DMA
depends on ARCH_AT91 || COMPILE_TEST
-   depends on HAS_DMA
select VIDEOBUF2_DMA_CONTIG
---help---
  This module makes the ATMEL Image Sensor Interface available
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c 
b/drivers/media/platform/soc_camera/atmel-isi.c
index 46de657c3e6d..a6d60c2e207d 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -22,18 +22,22 @@
 #include 
 #include 
 #include 
-
-#include 
-#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
 #include 
+#include 
 
 #include "atmel-isi.h"
 
-#define MAX_BUFFER_NUM 32
 #define MAX_SUPPORT_WIDTH  2048
 #define MAX_SUPPORT_HEIGHT 2048
-#define VID_LIMIT_BYTES(16 * 1024 * 1024)
 #define MIN_FRAME_RATE 15
 #define FRAME_INTERVAL_MILLI_SEC   (1000 / MIN_FRAME_RATE)
 
@@ -65,9 +69,37 @@ struct frame_buffer {
struct list_head list;
 };
 
+struct isi_graph_entity {
+   struct device_node *node;
+
+   struct v4l2_async_subdev asd;
+   struct v4l2_subdev *subdev;
+};
+
+/*
+ * struct isi_format - ISI media bus format information
+ * @fourcc:Fourcc code for this format
+ * @mbus_code: V4L2 media bus format code.
+ * @bpp:   Bytes per pixel (when stored in memory)
+ * @swap:  Byte swap configuration value
+ * @support:   Indicates format supported by subdev
+ * @skip:  Skip duplicate format supported by subdev
+ */
+struct isi_format {
+   u32 fourcc;
+   u32 mbus_code;
+   u8  bpp;
+   u32 swap;
+
+   boolsupport;
+   boolskip;
+};
+
+
 struct atmel_isi {
/* Protects the access of variables shared with the ISR */
-   spinlock_t  lock;
+   spinlock_t  irqlock;
+   struct device   *dev;
void __iomem*regs;
 
int sequence;
@@ -76,7 +108,7 @@ struct atmel_isi {
struct fbd  *p_fb_descriptors;
dma_addr_t  fb_descriptors_phys;
struct  list_head dma_desc_head;
-   struct isi_dma_desc dma_desc[MAX_BUFFER_NUM];
+   struct isi_dma_desc dma_desc[VIDEO_MAX_FRAME];
boolenable_preview_path;
 
struct completion   complete;
@@ -90,9 +122,22 @@ struct atmel_isi {
struct list_headvideo_buffer_list;
struct frame_buffer *active;
 
-   struct soc_camera_host  soc_host;
+   struct v4l2_device  v4l2_dev;
+   struct video_device *vdev;
+   struct v4l2_async_notifier  notifier;
+   struct isi_graph_entity entity;
+   struct v4l2_format  fmt;
+
+   struct isi_format   **user_formats;
+   unsigned intnum_user_formats;
+   const struct isi_format *current_fmt;
+
+   struct mutexlock;
+   struct vb2_queuequeue;
 };
 
+#define notifier_to_isi(n) container_of(n, struct atmel_isi, notifier)
+
 static void isi_writel(struct atmel_isi *isi, u32 reg, u32 val)
 {
writel(val, isi->regs + reg);
@@ -102,107 +147,46 @@ static u32 isi_readl(struct atmel_isi *isi, u32 reg)
return readl(isi->regs + reg);
 }
 
-static u32 setup_cfg2_yuv_swap(struct atmel_isi *isi,
-   const struct soc_camera_format_xlate *xlate)
-{
-   if (xlate->host_fmt->fourcc == V4L2_PIX_FMT_YUYV) {
-   /* all convert to YUYV */
-   switch (xlate->code) {
-   case MEDIA_BUS_FMT_VYUY8_2X8:
-   return ISI_CFG2_YCC_SWAP_MODE_3;
-   case MEDIA_BUS_FMT_UYVY8_2X8:
-   return ISI_CFG2_YCC_SWAP_MODE_2;
-   case MEDIA_BUS_FMT_YVYU8_2X8:
-   return

Re: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

Re: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

Re: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

Re: [PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

[PATCHv5 07/16] atmel-isi: remove dependency of the soc-camera framework

5 matches

Site Navigation

Mail list logo

Footer information