On 09/05/2016 10:24 AM, Hans Verkuil wrote:
> On 08/29/2016 03:21 PM, Jean-Christophe Trotin wrote:
>> This patch adds V4L2 HVA (Hardware Video Accelerator) video encoder
>> driver for STMicroelectronics SoC. It uses the V4L2 mem2mem framework.
>>
>> This patch only contains the core parts of the driver:
>> - the V4L2 interface with the userland (hva-v4l2.c)
>> - the hardware services (hva-hw.c)
>> - the memory management utilities (hva-mem.c)
>>
>> This patch doesn't include the support of specific codec (e.g. H.264)
>> video encoding: this support is part of subsequent patches.
>>
>> Signed-off-by: Yannick Fertre <yannick.fertre@xxxxxx>
>> Signed-off-by: Jean-Christophe Trotin <jean-christophe.trotin@xxxxxx>
>> ---
>> drivers/media/platform/Kconfig | 14 +
>> drivers/media/platform/Makefile | 1 +
>> drivers/media/platform/sti/hva/Makefile | 2 +
>> drivers/media/platform/sti/hva/hva-hw.c | 538 ++++++++++++
>> drivers/media/platform/sti/hva/hva-hw.h | 42 +
>> drivers/media/platform/sti/hva/hva-mem.c | 59 ++
>> drivers/media/platform/sti/hva/hva-mem.h | 34 +
>> drivers/media/platform/sti/hva/hva-v4l2.c | 1296 +++++++++++++++++++++++++++++
>> drivers/media/platform/sti/hva/hva.h | 290 +++++++
>> 9 files changed, 2276 insertions(+)
>> create mode 100644 drivers/media/platform/sti/hva/Makefile
>> create mode 100644 drivers/media/platform/sti/hva/hva-hw.c
>> create mode 100644 drivers/media/platform/sti/hva/hva-hw.h
>> create mode 100644 drivers/media/platform/sti/hva/hva-mem.c
>> create mode 100644 drivers/media/platform/sti/hva/hva-mem.h
>> create mode 100644 drivers/media/platform/sti/hva/hva-v4l2.c
>> create mode 100644 drivers/media/platform/sti/hva/hva.h
>>
>
> <snip>
>
>> +static int hva_s_parm(struct file *file, void *fh, struct v4l2_streamparm *sp)
>> +{
>> + struct hva_ctx *ctx = fh_to_ctx(file->private_data);
>> + struct v4l2_fract *time_per_frame = &ctx->ctrls.time_per_frame;
>> +
>> + time_per_frame->numerator = sp->parm.capture.timeperframe.numerator;
>> + time_per_frame->denominator =
>> + sp->parm.capture.timeperframe.denominator;
>> +
>> + return 0;
>> +}
>> +
>> +static int hva_g_parm(struct file *file, void *fh, struct v4l2_streamparm *sp)
>> +{
>> + struct hva_ctx *ctx = fh_to_ctx(file->private_data);
>> + struct v4l2_fract *time_per_frame = &ctx->ctrls.time_per_frame;
>> +
>> + sp->parm.capture.timeperframe.numerator = time_per_frame->numerator;
>> + sp->parm.capture.timeperframe.denominator =
>> + time_per_frame->denominator;
>> +
>> + return 0;
>> +}
>
> In this implementation g/s_parm is supported for both capture and output. Is that
> intended? If so, please add a comment. If not, then you should check the type.
>
> Also the V4L2_CAP_TIMEPERFRAME capability isn't set. I've just added a check to
> v4l2-compliance to test for that.
>
> As per the kbuild robot report you also need to depend on HAS_DMA in the Kconfig.
>
> I have no other comments, so once these comments are fixed I can make a pull request.
>
> Making a v6 should be quick: if you can post v6 today, then I would very much appreciate
> it.
>
> Regards,
>
> Hans
>
Hi Hans,
I've aligned the implementation g/s parm with the ones available in coda and
mtk-vcodec: g/s parm is supported for output and is rejected (-EINVAL) for any
other type.
I'm confused with the V4L2_CAP_TIMEPERFRAME capability: my understanding is that
it should be set only in g_parm (as it's presently done in coda and mtk-vcodec).
But, doing this way, then there's a warning with the v4l2-compliance test that
you add. Indeed, this test checks the capability after a call to s_parm (which
would mean that s_parm should also set the capability), and moreover, always
checks it for capture (parm.parm.capture.capability) even the type is output.
Could you bring me some explanation about this point (either by email or by IRC)?
The problem reported by the kbuild robot ("depends on HAS_DMA" missing in the
Kconfig) will also be corrected in the version 6.
I'm ready to deliver a version 6 today: the only remaining point is about the
V4L2_CAP_TIMEPERFRAME capability as explained above.
Regards,
Jean-Christophe.--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
