Re: [PATCH] media: stm32-dcmi: add JPEG support

[Hugues FRUCHET]

Hi Hans,

Yes depends on 'fix lock scheme', I'll send a v2 of this jpeg commit for 
unneeded V4L2_FMT_FLAG_COMPRESSED.

Best regards,
Hugues.

On 02/26/2018 03:17 PM, Hans Verkuil wrote:
> On 02/22/2018 10:51 AM, Hugues Fruchet wrote:
>> Add DCMI JPEG support.
> 
> Does this patch depend on the 'fix lock scheme' patch?
> 
> It looks good to me except for one small thing (see below), but I think this
> depends on the 'fix lock scheme' patch, right?
> 
>>
>> Signed-off-by: Hugues Fruchet 
>> ---
>>   drivers/media/platform/stm32/stm32-dcmi.c | 195 
>> +++---
>>   1 file changed, 148 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
>> b/drivers/media/platform/stm32/stm32-dcmi.c
>> index 269e963..7eaaf7c 100644
>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>> @@ -93,6 +93,11 @@ enum state {
>>   #define MIN_HEIGHT 16U
>>   #define MAX_HEIGHT 2048U
>>   
>> +#define MIN_JPEG_WIDTH  16U
>> +#define MAX_JPEG_WIDTH  2592U
>> +#define MIN_JPEG_HEIGHT 16U
>> +#define MAX_JPEG_HEIGHT 2592U
>> +
>>   #define TIMEOUT_MS 1000
>>   
>>   struct dcmi_graph_entity {
>> @@ -191,14 +196,67 @@ static inline void reg_clear(void __iomem *base, u32 
>> reg, u32 mask)
>>   
>>   static int dcmi_start_capture(struct stm32_dcmi *dcmi);
>>   
>> +static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
>> + struct dcmi_buf *buf,
>> + size_t bytesused,
>> + int err)
>> +{
>> +struct vb2_v4l2_buffer *vbuf;
>> +
>> +if (!buf)
>> +return;
>> +
>> +vbuf = >vb;
>> +
>> +vbuf->sequence = dcmi->sequence++;
>> +vbuf->field = V4L2_FIELD_NONE;
>> +vbuf->vb2_buf.timestamp = ktime_get_ns();
>> +vb2_set_plane_payload(>vb2_buf, 0, bytesused);
>> +vb2_buffer_done(>vb2_buf,
>> +err ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
>> +dev_dbg(dcmi->dev, "buffer[%d] done seq=%d, bytesused=%zu\n",
>> +vbuf->vb2_buf.index, vbuf->sequence, bytesused);
>> +
>> +dcmi->buffers_count++;
>> +dcmi->active = NULL;
>> +}
>> +
>> +static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
>> +{
>> +spin_lock_irq(>irqlock);
>> +
>> +if (dcmi->state != RUNNING) {
>> +spin_unlock_irq(>irqlock);
>> +return -EINVAL;
>> +}
>> +
>> +/* Restart a new DMA transfer with next buffer */
>> +if (list_empty(>buffers)) {
>> +dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture 
>> buffer\n",
>> +__func__);
>> +dcmi->errors_count++;
>> +dcmi->active = NULL;
>> +
>> +spin_unlock_irq(>irqlock);
>> +return -EINVAL;
>> +}
>> +
>> +dcmi->active = list_entry(dcmi->buffers.next,
>> +  struct dcmi_buf, list);
>> +list_del_init(>active->list);
>> +
>> +spin_unlock_irq(>irqlock);
>> +
>> +return dcmi_start_capture(dcmi);
>> +}
>> +
>>   static void dcmi_dma_callback(void *param)
>>   {
>>  struct stm32_dcmi *dcmi = (struct stm32_dcmi *)param;
>>  struct dma_chan *chan = dcmi->dma_chan;
>>  struct dma_tx_state state;
>>  enum dma_status status;
>> -
>> -spin_lock_irq(>irqlock);
>> +struct dcmi_buf *buf = dcmi->active;
>>   
>>  /* Check DMA status */
>>  status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
>> @@ -216,53 +274,18 @@ static void dcmi_dma_callback(void *param)
>>  case DMA_COMPLETE:
>>  dev_dbg(dcmi->dev, "%s: Received DMA_COMPLETE\n", __func__);
>>   
>> -if (dcmi->active) {
>> -struct dcmi_buf *buf = dcmi->active;
>> -struct vb2_v4l2_buffer *vbuf = >active->vb;
>> -
>> -vbuf->sequence = dcmi->sequence++;
>> -vbuf->field = V4L2_FIELD_NONE;
>> -vbuf->vb2_buf.timestamp = ktime_get_ns();
>> -vb2_set_plane_payload(>vb2_buf, 0, buf->size);
>> -vb2_buffer_done(>vb2_buf, VB2_BUF_STATE_DONE);
>> -dev_dbg(dcmi->dev, "buffer[%d] done seq=%d\n",
>> -vbuf->vb2_buf.index, vbuf->sequence);
>> -
>> -dcmi->buffers_count++;
>> -dcmi->active = NULL;
>> -}
>> -
>> -/* Restart a new DMA transfer with next buffer */
>> -if (dcmi->state == RUNNING) {
>> -if (list_empty(>buffers)) {
>> -dev_err(dcmi->dev, "%s: No more buffer queued, 
>> cannot capture buffer\n",
>> -__func__);
>> -dcmi->errors_count++;
>> -dcmi->active = NULL;
>> -
>> -spin_unlock_irq(>irqlock);
>> -return;
>> -}

Re: [PATCH] media: stm32-dcmi: add JPEG support

[Hans Verkuil]

On 02/22/2018 10:51 AM, Hugues Fruchet wrote:
> Add DCMI JPEG support.

Does this patch depend on the 'fix lock scheme' patch?

It looks good to me except for one small thing (see below), but I think this
depends on the 'fix lock scheme' patch, right?

> 
> Signed-off-by: Hugues Fruchet 
> ---
>  drivers/media/platform/stm32/stm32-dcmi.c | 195 
> +++---
>  1 file changed, 148 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
> b/drivers/media/platform/stm32/stm32-dcmi.c
> index 269e963..7eaaf7c 100644
> --- a/drivers/media/platform/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
> @@ -93,6 +93,11 @@ enum state {
>  #define MIN_HEIGHT   16U
>  #define MAX_HEIGHT   2048U
>  
> +#define MIN_JPEG_WIDTH   16U
> +#define MAX_JPEG_WIDTH   2592U
> +#define MIN_JPEG_HEIGHT  16U
> +#define MAX_JPEG_HEIGHT  2592U
> +
>  #define TIMEOUT_MS   1000
>  
>  struct dcmi_graph_entity {
> @@ -191,14 +196,67 @@ static inline void reg_clear(void __iomem *base, u32 
> reg, u32 mask)
>  
>  static int dcmi_start_capture(struct stm32_dcmi *dcmi);
>  
> +static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
> +  struct dcmi_buf *buf,
> +  size_t bytesused,
> +  int err)
> +{
> + struct vb2_v4l2_buffer *vbuf;
> +
> + if (!buf)
> + return;
> +
> + vbuf = >vb;
> +
> + vbuf->sequence = dcmi->sequence++;
> + vbuf->field = V4L2_FIELD_NONE;
> + vbuf->vb2_buf.timestamp = ktime_get_ns();
> + vb2_set_plane_payload(>vb2_buf, 0, bytesused);
> + vb2_buffer_done(>vb2_buf,
> + err ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
> + dev_dbg(dcmi->dev, "buffer[%d] done seq=%d, bytesused=%zu\n",
> + vbuf->vb2_buf.index, vbuf->sequence, bytesused);
> +
> + dcmi->buffers_count++;
> + dcmi->active = NULL;
> +}
> +
> +static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
> +{
> + spin_lock_irq(>irqlock);
> +
> + if (dcmi->state != RUNNING) {
> + spin_unlock_irq(>irqlock);
> + return -EINVAL;
> + }
> +
> + /* Restart a new DMA transfer with next buffer */
> + if (list_empty(>buffers)) {
> + dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture 
> buffer\n",
> + __func__);
> + dcmi->errors_count++;
> + dcmi->active = NULL;
> +
> + spin_unlock_irq(>irqlock);
> + return -EINVAL;
> + }
> +
> + dcmi->active = list_entry(dcmi->buffers.next,
> +   struct dcmi_buf, list);
> + list_del_init(>active->list);
> +
> + spin_unlock_irq(>irqlock);
> +
> + return dcmi_start_capture(dcmi);
> +}
> +
>  static void dcmi_dma_callback(void *param)
>  {
>   struct stm32_dcmi *dcmi = (struct stm32_dcmi *)param;
>   struct dma_chan *chan = dcmi->dma_chan;
>   struct dma_tx_state state;
>   enum dma_status status;
> -
> - spin_lock_irq(>irqlock);
> + struct dcmi_buf *buf = dcmi->active;
>  
>   /* Check DMA status */
>   status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
> @@ -216,53 +274,18 @@ static void dcmi_dma_callback(void *param)
>   case DMA_COMPLETE:
>   dev_dbg(dcmi->dev, "%s: Received DMA_COMPLETE\n", __func__);
>  
> - if (dcmi->active) {
> - struct dcmi_buf *buf = dcmi->active;
> - struct vb2_v4l2_buffer *vbuf = >active->vb;
> -
> - vbuf->sequence = dcmi->sequence++;
> - vbuf->field = V4L2_FIELD_NONE;
> - vbuf->vb2_buf.timestamp = ktime_get_ns();
> - vb2_set_plane_payload(>vb2_buf, 0, buf->size);
> - vb2_buffer_done(>vb2_buf, VB2_BUF_STATE_DONE);
> - dev_dbg(dcmi->dev, "buffer[%d] done seq=%d\n",
> - vbuf->vb2_buf.index, vbuf->sequence);
> -
> - dcmi->buffers_count++;
> - dcmi->active = NULL;
> - }
> -
> - /* Restart a new DMA transfer with next buffer */
> - if (dcmi->state == RUNNING) {
> - if (list_empty(>buffers)) {
> - dev_err(dcmi->dev, "%s: No more buffer queued, 
> cannot capture buffer\n",
> - __func__);
> - dcmi->errors_count++;
> - dcmi->active = NULL;
> -
> - spin_unlock_irq(>irqlock);
> - return;
> - }
> -
> - dcmi->active = list_entry(dcmi->buffers.next,
> -   struct dcmi_buf, list);
> -
> - list_del_init(>active->list);
> -
> - spin_unlock_irq(>irqlock);
> 

[PATCH] media: stm32-dcmi: add JPEG support

[Hugues Fruchet]

Add DCMI JPEG support.

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

diff --git a/drivers/media/platform/stm32/stm32-dcmi.c 
b/drivers/media/platform/stm32/stm32-dcmi.c
index 269e963..7eaaf7c 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -93,6 +93,11 @@ enum state {
 #define MIN_HEIGHT 16U
 #define MAX_HEIGHT 2048U
 
+#define MIN_JPEG_WIDTH 16U
+#define MAX_JPEG_WIDTH 2592U
+#define MIN_JPEG_HEIGHT16U
+#define MAX_JPEG_HEIGHT2592U
+
 #define TIMEOUT_MS 1000
 
 struct dcmi_graph_entity {
@@ -191,14 +196,67 @@ static inline void reg_clear(void __iomem *base, u32 reg, 
u32 mask)
 
 static int dcmi_start_capture(struct stm32_dcmi *dcmi);
 
+static void dcmi_buffer_done(struct stm32_dcmi *dcmi,
+struct dcmi_buf *buf,
+size_t bytesused,
+int err)
+{
+   struct vb2_v4l2_buffer *vbuf;
+
+   if (!buf)
+   return;
+
+   vbuf = >vb;
+
+   vbuf->sequence = dcmi->sequence++;
+   vbuf->field = V4L2_FIELD_NONE;
+   vbuf->vb2_buf.timestamp = ktime_get_ns();
+   vb2_set_plane_payload(>vb2_buf, 0, bytesused);
+   vb2_buffer_done(>vb2_buf,
+   err ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
+   dev_dbg(dcmi->dev, "buffer[%d] done seq=%d, bytesused=%zu\n",
+   vbuf->vb2_buf.index, vbuf->sequence, bytesused);
+
+   dcmi->buffers_count++;
+   dcmi->active = NULL;
+}
+
+static int dcmi_restart_capture(struct stm32_dcmi *dcmi)
+{
+   spin_lock_irq(>irqlock);
+
+   if (dcmi->state != RUNNING) {
+   spin_unlock_irq(>irqlock);
+   return -EINVAL;
+   }
+
+   /* Restart a new DMA transfer with next buffer */
+   if (list_empty(>buffers)) {
+   dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture 
buffer\n",
+   __func__);
+   dcmi->errors_count++;
+   dcmi->active = NULL;
+
+   spin_unlock_irq(>irqlock);
+   return -EINVAL;
+   }
+
+   dcmi->active = list_entry(dcmi->buffers.next,
+ struct dcmi_buf, list);
+   list_del_init(>active->list);
+
+   spin_unlock_irq(>irqlock);
+
+   return dcmi_start_capture(dcmi);
+}
+
 static void dcmi_dma_callback(void *param)
 {
struct stm32_dcmi *dcmi = (struct stm32_dcmi *)param;
struct dma_chan *chan = dcmi->dma_chan;
struct dma_tx_state state;
enum dma_status status;
-
-   spin_lock_irq(>irqlock);
+   struct dcmi_buf *buf = dcmi->active;
 
/* Check DMA status */
status = dmaengine_tx_status(chan, dcmi->dma_cookie, );
@@ -216,53 +274,18 @@ static void dcmi_dma_callback(void *param)
case DMA_COMPLETE:
dev_dbg(dcmi->dev, "%s: Received DMA_COMPLETE\n", __func__);
 
-   if (dcmi->active) {
-   struct dcmi_buf *buf = dcmi->active;
-   struct vb2_v4l2_buffer *vbuf = >active->vb;
-
-   vbuf->sequence = dcmi->sequence++;
-   vbuf->field = V4L2_FIELD_NONE;
-   vbuf->vb2_buf.timestamp = ktime_get_ns();
-   vb2_set_plane_payload(>vb2_buf, 0, buf->size);
-   vb2_buffer_done(>vb2_buf, VB2_BUF_STATE_DONE);
-   dev_dbg(dcmi->dev, "buffer[%d] done seq=%d\n",
-   vbuf->vb2_buf.index, vbuf->sequence);
-
-   dcmi->buffers_count++;
-   dcmi->active = NULL;
-   }
-
-   /* Restart a new DMA transfer with next buffer */
-   if (dcmi->state == RUNNING) {
-   if (list_empty(>buffers)) {
-   dev_err(dcmi->dev, "%s: No more buffer queued, 
cannot capture buffer\n",
-   __func__);
-   dcmi->errors_count++;
-   dcmi->active = NULL;
-
-   spin_unlock_irq(>irqlock);
-   return;
-   }
-
-   dcmi->active = list_entry(dcmi->buffers.next,
- struct dcmi_buf, list);
-
-   list_del_init(>active->list);
-
-   spin_unlock_irq(>irqlock);
-   if (dcmi_start_capture(dcmi))
-   dev_err(dcmi->dev, "%s: Cannot restart capture 
on DMA complete\n",
-   __func__);
-   return;
-   }
+   /* Return buffer to V4L2 */
+   dcmi_buffer_done(dcmi, buf, buf->size, 0);