subject:"\[PATCH v3 1\/3\] st\-hva\: encoding summary at instance release"

Re: [PATCH v3 1/3] st-hva: encoding summary at instance release

2017-01-31 Thread Mauro Carvalho Chehab

Em Tue, 31 Jan 2017 08:50:38 +
Jean Christophe TROTIN  escreveu:

> On 01/30/2017 06:28 PM, Mauro Carvalho Chehab wrote:
> > Em Mon, 28 Nov 2016 11:30:52 +0100
> > Jean-Christophe Trotin  escreveu:
> >  
> >> This patch prints unconditionnaly a short summary  
> >
> > Why? Is this driver so broken that everyone would need an
> > unconditional "short summary" about what happened there?
> >
> > If not, then please use dev_dbg() or debugfs instead. If yes, then
> > we should move this driver to staging.
> >  
> Hi Mauro,
> 
> The unconditional character of this "short summary" was a "facility" that our 
> customers used to get (it doesn't mean that this driver is broken).
> That's said, I understand your comment, and I will propose today a new 
> version 
> of this patch with dev_dbg() instead of dev_info().

Thanks, the new version looks OK on my eyes. As I got it from Hans
tree, I'll wait for either his ack/SOB or for him to merge on his
tree.

Regards,
Mauro

> 
> Regards,
> Jean-Christophe.
> 
> >> about the encoding
> >> operation at each instance closing, for debug purpose:
> >> - information about the frame (format, resolution)
> >> - information about the stream (format, profile, level, resolution)
> >> - number of encoded frames
> >> - potential (system, encoding...) errors
> >>
> >> Signed-off-by: Yannick Fertre 
> >> Signed-off-by: Jean-Christophe Trotin 
> >> ---
> >>  drivers/media/platform/sti/hva/hva-h264.c |  6 
> >>  drivers/media/platform/sti/hva/hva-hw.c   |  5 
> >>  drivers/media/platform/sti/hva/hva-mem.c  |  5 +++-
> >>  drivers/media/platform/sti/hva/hva-v4l2.c | 49 
> >> ---
> >>  drivers/media/platform/sti/hva/hva.h  |  8 +
> >>  5 files changed, 62 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/sti/hva/hva-h264.c 
> >> b/drivers/media/platform/sti/hva/hva-h264.c
> >> index 8cc8467..e6f247a 100644
> >> --- a/drivers/media/platform/sti/hva/hva-h264.c
> >> +++ b/drivers/media/platform/sti/hva/hva-h264.c
> >> @@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
> >>"%s   width(%d) or height(%d) exceeds limits (%dx%d)\n",
> >>pctx->name, frame_width, frame_height,
> >>H264_MAX_SIZE_W, H264_MAX_SIZE_H);
> >> +  pctx->frame_errors++;
> >>return -EINVAL;
> >>}
> >>
> >> @@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
> >>default:
> >>dev_err(dev, "%s   invalid source pixel format\n",
> >>pctx->name);
> >> +  pctx->frame_errors++;
> >>return -EINVAL;
> >>}
> >>
> >> @@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
> >>
> >>if (td->framerate_den == 0) {
> >>dev_err(dev, "%s   invalid framerate\n", pctx->name);
> >> +  pctx->frame_errors++;
> >>return -EINVAL;
> >>}
> >>
> >> @@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
> >>(payload > MAX_SPS_PPS_SIZE)) {
> >>dev_err(dev, "%s   invalid sps/pps size %d\n", pctx->name,
> >>payload);
> >> +  pctx->frame_errors++;
> >>return -EINVAL;
> >>}
> >>
> >> @@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
> >>   (u8 *)stream->vaddr,
> >>   )) {
> >>dev_err(dev, "%s   fail to get SEI nal\n", pctx->name);
> >> +  pctx->frame_errors++;
> >>return -EINVAL;
> >>}
> >>
> >> @@ -963,6 +968,7 @@ static int hva_h264_open(struct hva_ctx *pctx)
> >>  err_ctx:
> >>devm_kfree(dev, ctx);
> >>  err:
> >> +  pctx->sys_errors++;
> >>return ret;
> >>  }
> >>
> >> diff --git a/drivers/media/platform/sti/hva/hva-hw.c 
> >> b/drivers/media/platform/sti/hva/hva-hw.c
> >> index 68d625b..5104068 100644
> >> --- a/drivers/media/platform/sti/hva/hva-hw.c
> >> +++ b/drivers/media/platform/sti/hva/hva-hw.c
> >> @@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
> >> hva_hw_cmd_type cmd,
> >>
> >>if (pm_runtime_get_sync(dev) < 0) {
> >>dev_err(dev, "%s failed to get pm_runtime\n", ctx->name);
> >> +  ctx->sys_errors++;
> >>ret = -EFAULT;
> >>goto out;
> >>}
> >> @@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
> >> hva_hw_cmd_type cmd,
> >>break;
> >>default:
> >>dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd);
> >> +  ctx->encode_errors++;
> >>ret = -EFAULT;
> >>goto out;
> >>}
> >> @@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
> >> hva_hw_cmd_type cmd,
> >>

Re: [PATCH v3 1/3] st-hva: encoding summary at instance release

2017-01-31 Thread Jean Christophe TROTIN



On 01/30/2017 06:28 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 28 Nov 2016 11:30:52 +0100
> Jean-Christophe Trotin  escreveu:
>
>> This patch prints unconditionnaly a short summary
>
> Why? Is this driver so broken that everyone would need an
> unconditional "short summary" about what happened there?
>
> If not, then please use dev_dbg() or debugfs instead. If yes, then
> we should move this driver to staging.
>
Hi Mauro,

The unconditional character of this "short summary" was a "facility" that our 
customers used to get (it doesn't mean that this driver is broken).
That's said, I understand your comment, and I will propose today a new version 
of this patch with dev_dbg() instead of dev_info().

Regards,
Jean-Christophe.

>> about the encoding
>> operation at each instance closing, for debug purpose:
>> - information about the frame (format, resolution)
>> - information about the stream (format, profile, level, resolution)
>> - number of encoded frames
>> - potential (system, encoding...) errors
>>
>> Signed-off-by: Yannick Fertre 
>> Signed-off-by: Jean-Christophe Trotin 
>> ---
>>  drivers/media/platform/sti/hva/hva-h264.c |  6 
>>  drivers/media/platform/sti/hva/hva-hw.c   |  5 
>>  drivers/media/platform/sti/hva/hva-mem.c  |  5 +++-
>>  drivers/media/platform/sti/hva/hva-v4l2.c | 49 
>> ---
>>  drivers/media/platform/sti/hva/hva.h  |  8 +
>>  5 files changed, 62 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/platform/sti/hva/hva-h264.c 
>> b/drivers/media/platform/sti/hva/hva-h264.c
>> index 8cc8467..e6f247a 100644
>> --- a/drivers/media/platform/sti/hva/hva-h264.c
>> +++ b/drivers/media/platform/sti/hva/hva-h264.c
>> @@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>>  "%s   width(%d) or height(%d) exceeds limits (%dx%d)\n",
>>  pctx->name, frame_width, frame_height,
>>  H264_MAX_SIZE_W, H264_MAX_SIZE_H);
>> +pctx->frame_errors++;
>>  return -EINVAL;
>>  }
>>
>> @@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>>  default:
>>  dev_err(dev, "%s   invalid source pixel format\n",
>>  pctx->name);
>> +pctx->frame_errors++;
>>  return -EINVAL;
>>  }
>>
>> @@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>>
>>  if (td->framerate_den == 0) {
>>  dev_err(dev, "%s   invalid framerate\n", pctx->name);
>> +pctx->frame_errors++;
>>  return -EINVAL;
>>  }
>>
>> @@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>>  (payload > MAX_SPS_PPS_SIZE)) {
>>  dev_err(dev, "%s   invalid sps/pps size %d\n", pctx->name,
>>  payload);
>> +pctx->frame_errors++;
>>  return -EINVAL;
>>  }
>>
>> @@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>> (u8 *)stream->vaddr,
>> )) {
>>  dev_err(dev, "%s   fail to get SEI nal\n", pctx->name);
>> +pctx->frame_errors++;
>>  return -EINVAL;
>>  }
>>
>> @@ -963,6 +968,7 @@ static int hva_h264_open(struct hva_ctx *pctx)
>>  err_ctx:
>>  devm_kfree(dev, ctx);
>>  err:
>> +pctx->sys_errors++;
>>  return ret;
>>  }
>>
>> diff --git a/drivers/media/platform/sti/hva/hva-hw.c 
>> b/drivers/media/platform/sti/hva/hva-hw.c
>> index 68d625b..5104068 100644
>> --- a/drivers/media/platform/sti/hva/hva-hw.c
>> +++ b/drivers/media/platform/sti/hva/hva-hw.c
>> @@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
>> hva_hw_cmd_type cmd,
>>
>>  if (pm_runtime_get_sync(dev) < 0) {
>>  dev_err(dev, "%s failed to get pm_runtime\n", ctx->name);
>> +ctx->sys_errors++;
>>  ret = -EFAULT;
>>  goto out;
>>  }
>> @@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
>> hva_hw_cmd_type cmd,
>>  break;
>>  default:
>>  dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd);
>> +ctx->encode_errors++;
>>  ret = -EFAULT;
>>  goto out;
>>  }
>> @@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
>> hva_hw_cmd_type cmd,
>>   msecs_to_jiffies(2000))) {
>>  dev_err(dev, "%s %s: time out on completion\n", ctx->name,
>>  __func__);
>> +ctx->encode_errors++;
>>  ret = -EFAULT;
>>  goto out;
>>  }
>> @@ -518,6 +521,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
>> hva_hw_cmd_type cmd,
>>  /* get encoding status */
>>  ret = ctx->hw_err ?

Re: [PATCH v3 1/3] st-hva: encoding summary at instance release

2017-01-30 Thread Mauro Carvalho Chehab

Em Mon, 28 Nov 2016 11:30:52 +0100
Jean-Christophe Trotin  escreveu:

> This patch prints unconditionnaly a short summary 

Why? Is this driver so broken that everyone would need an
unconditional "short summary" about what happened there?

If not, then please use dev_dbg() or debugfs instead. If yes, then
we should move this driver to staging.

> about the encoding
> operation at each instance closing, for debug purpose:
> - information about the frame (format, resolution)
> - information about the stream (format, profile, level, resolution)
> - number of encoded frames
> - potential (system, encoding...) errors
> 
> Signed-off-by: Yannick Fertre 
> Signed-off-by: Jean-Christophe Trotin 
> ---
>  drivers/media/platform/sti/hva/hva-h264.c |  6 
>  drivers/media/platform/sti/hva/hva-hw.c   |  5 
>  drivers/media/platform/sti/hva/hva-mem.c  |  5 +++-
>  drivers/media/platform/sti/hva/hva-v4l2.c | 49 
> ---
>  drivers/media/platform/sti/hva/hva.h  |  8 +
>  5 files changed, 62 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/sti/hva/hva-h264.c 
> b/drivers/media/platform/sti/hva/hva-h264.c
> index 8cc8467..e6f247a 100644
> --- a/drivers/media/platform/sti/hva/hva-h264.c
> +++ b/drivers/media/platform/sti/hva/hva-h264.c
> @@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>   "%s   width(%d) or height(%d) exceeds limits (%dx%d)\n",
>   pctx->name, frame_width, frame_height,
>   H264_MAX_SIZE_W, H264_MAX_SIZE_H);
> + pctx->frame_errors++;
>   return -EINVAL;
>   }
>  
> @@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>   default:
>   dev_err(dev, "%s   invalid source pixel format\n",
>   pctx->name);
> + pctx->frame_errors++;
>   return -EINVAL;
>   }
>  
> @@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>  
>   if (td->framerate_den == 0) {
>   dev_err(dev, "%s   invalid framerate\n", pctx->name);
> + pctx->frame_errors++;
>   return -EINVAL;
>   }
>  
> @@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>   (payload > MAX_SPS_PPS_SIZE)) {
>   dev_err(dev, "%s   invalid sps/pps size %d\n", pctx->name,
>   payload);
> + pctx->frame_errors++;
>   return -EINVAL;
>   }
>  
> @@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>  (u8 *)stream->vaddr,
>  )) {
>   dev_err(dev, "%s   fail to get SEI nal\n", pctx->name);
> + pctx->frame_errors++;
>   return -EINVAL;
>   }
>  
> @@ -963,6 +968,7 @@ static int hva_h264_open(struct hva_ctx *pctx)
>  err_ctx:
>   devm_kfree(dev, ctx);
>  err:
> + pctx->sys_errors++;
>   return ret;
>  }
>  
> diff --git a/drivers/media/platform/sti/hva/hva-hw.c 
> b/drivers/media/platform/sti/hva/hva-hw.c
> index 68d625b..5104068 100644
> --- a/drivers/media/platform/sti/hva/hva-hw.c
> +++ b/drivers/media/platform/sti/hva/hva-hw.c
> @@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
> hva_hw_cmd_type cmd,
>  
>   if (pm_runtime_get_sync(dev) < 0) {
>   dev_err(dev, "%s failed to get pm_runtime\n", ctx->name);
> + ctx->sys_errors++;
>   ret = -EFAULT;
>   goto out;
>   }
> @@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
> hva_hw_cmd_type cmd,
>   break;
>   default:
>   dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd);
> + ctx->encode_errors++;
>   ret = -EFAULT;
>   goto out;
>   }
> @@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
> hva_hw_cmd_type cmd,
>msecs_to_jiffies(2000))) {
>   dev_err(dev, "%s %s: time out on completion\n", ctx->name,
>   __func__);
> + ctx->encode_errors++;
>   ret = -EFAULT;
>   goto out;
>   }
> @@ -518,6 +521,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
> hva_hw_cmd_type cmd,
>   /* get encoding status */
>   ret = ctx->hw_err ? -EFAULT : 0;
>  
> + ctx->encode_errors += ctx->hw_err ? 1 : 0;
> +
>  out:
>   disable_irq(hva->irq_its);
>   disable_irq(hva->irq_err);
> diff --git a/drivers/media/platform/sti/hva/hva-mem.c 
> b/drivers/media/platform/sti/hva/hva-mem.c
> index 649f660..821c78e 100644
> --- a/drivers/media/platform/sti/hva/hva-mem.c
> +++ b/drivers/media/platform/sti/hva/hva-mem.c
> @@ -17,14 +17,17 @@ int hva_mem_alloc(struct

[PATCH v3 1/3] st-hva: encoding summary at instance release

2016-11-28 Thread Jean-Christophe Trotin

This patch prints unconditionnaly a short summary about the encoding
operation at each instance closing, for debug purpose:
- information about the frame (format, resolution)
- information about the stream (format, profile, level, resolution)
- number of encoded frames
- potential (system, encoding...) errors

Signed-off-by: Yannick Fertre 
Signed-off-by: Jean-Christophe Trotin 
---
 drivers/media/platform/sti/hva/hva-h264.c |  6 
 drivers/media/platform/sti/hva/hva-hw.c   |  5 
 drivers/media/platform/sti/hva/hva-mem.c  |  5 +++-
 drivers/media/platform/sti/hva/hva-v4l2.c | 49 ---
 drivers/media/platform/sti/hva/hva.h  |  8 +
 5 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/sti/hva/hva-h264.c 
b/drivers/media/platform/sti/hva/hva-h264.c
index 8cc8467..e6f247a 100644
--- a/drivers/media/platform/sti/hva/hva-h264.c
+++ b/drivers/media/platform/sti/hva/hva-h264.c
@@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
"%s   width(%d) or height(%d) exceeds limits (%dx%d)\n",
pctx->name, frame_width, frame_height,
H264_MAX_SIZE_W, H264_MAX_SIZE_H);
+   pctx->frame_errors++;
return -EINVAL;
}
 
@@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
default:
dev_err(dev, "%s   invalid source pixel format\n",
pctx->name);
+   pctx->frame_errors++;
return -EINVAL;
}
 
@@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
 
if (td->framerate_den == 0) {
dev_err(dev, "%s   invalid framerate\n", pctx->name);
+   pctx->frame_errors++;
return -EINVAL;
}
 
@@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
(payload > MAX_SPS_PPS_SIZE)) {
dev_err(dev, "%s   invalid sps/pps size %d\n", pctx->name,
payload);
+   pctx->frame_errors++;
return -EINVAL;
}
 
@@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
   (u8 *)stream->vaddr,
   )) {
dev_err(dev, "%s   fail to get SEI nal\n", pctx->name);
+   pctx->frame_errors++;
return -EINVAL;
}
 
@@ -963,6 +968,7 @@ static int hva_h264_open(struct hva_ctx *pctx)
 err_ctx:
devm_kfree(dev, ctx);
 err:
+   pctx->sys_errors++;
return ret;
 }
 
diff --git a/drivers/media/platform/sti/hva/hva-hw.c 
b/drivers/media/platform/sti/hva/hva-hw.c
index 68d625b..5104068 100644
--- a/drivers/media/platform/sti/hva/hva-hw.c
+++ b/drivers/media/platform/sti/hva/hva-hw.c
@@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
hva_hw_cmd_type cmd,
 
if (pm_runtime_get_sync(dev) < 0) {
dev_err(dev, "%s failed to get pm_runtime\n", ctx->name);
+   ctx->sys_errors++;
ret = -EFAULT;
goto out;
}
@@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
hva_hw_cmd_type cmd,
break;
default:
dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd);
+   ctx->encode_errors++;
ret = -EFAULT;
goto out;
}
@@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
hva_hw_cmd_type cmd,
 msecs_to_jiffies(2000))) {
dev_err(dev, "%s %s: time out on completion\n", ctx->name,
__func__);
+   ctx->encode_errors++;
ret = -EFAULT;
goto out;
}
@@ -518,6 +521,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum 
hva_hw_cmd_type cmd,
/* get encoding status */
ret = ctx->hw_err ? -EFAULT : 0;
 
+   ctx->encode_errors += ctx->hw_err ? 1 : 0;
+
 out:
disable_irq(hva->irq_its);
disable_irq(hva->irq_err);
diff --git a/drivers/media/platform/sti/hva/hva-mem.c 
b/drivers/media/platform/sti/hva/hva-mem.c
index 649f660..821c78e 100644
--- a/drivers/media/platform/sti/hva/hva-mem.c
+++ b/drivers/media/platform/sti/hva/hva-mem.c
@@ -17,14 +17,17 @@ int hva_mem_alloc(struct hva_ctx *ctx, u32 size, const char 
*name,
void *base;
 
b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL);
-   if (!b)
+   if (!b) {
+   ctx->sys_errors++;
return -ENOMEM;
+   }
 
base = dma_alloc_attrs(dev, size, , GFP_KERNEL | GFP_DMA,
   DMA_ATTR_WRITE_COMBINE);
if (!base) {
dev_err(dev, "%s %s : dma_alloc_attrs failed for %s

Re: [PATCH v3 1/3] st-hva: encoding summary at instance release

Re: [PATCH v3 1/3] st-hva: encoding summary at instance release

Re: [PATCH v3 1/3] st-hva: encoding summary at instance release

[PATCH v3 1/3] st-hva: encoding summary at instance release

4 matches

Site Navigation

Mail list logo

Footer information