[PATCH v2 0/7] media: mtk-vcodec: few fixes

[PATCH v2 0/7] media: mtk-vcodec: few fixes

[Dafna Hirschfeld]

This is v2 of patches that fixes issues in the mtk-vcodec encoder
v1 is [1]

patch 1 add support to vp8 profile contorl. Without this control the encoder can't be used with gstreamer.
patches 2-3 are bug fixes
patch 4: fixes the debugging
patch 5-7 are cleanups

changes from v1:
1. some cleanup patches (4-6) are new
2. improve commit log of patches.


[1] https://lore.kernel.org/linux-media/34a3f0e40c5248472d072d2a06cc4370e08ea9ff.camel@ndufresne.ca/T/

Dafna Hirschfeld (7):
  media: mtk-vcodec: enc: add vp8 profile ctrl
  media: mtk-vcodec: call v4l2_m2m_ctx_release first when file is
    released
  media: mtk-vcodec: enc: use "stream_started" flag for
    "stop/start_streaming"
  media: mtk-vcodec: fix debugging defines
  media: mtk-vcodec: replace func vidioc_try_fmt with two funcs for
    out/cap
  media: mtk-vcodec: don't check return val of mtk_venc_get_q_data
  meida: mtk-vcodec: remove unused func parameter

 .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c  |   3 -
 .../platform/mtk-vcodec/mtk_vcodec_drv.h      |   4 +
 .../platform/mtk-vcodec/mtk_vcodec_enc.c      | 206 +++++++++---------
 .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c  |   5 +-
 .../platform/mtk-vcodec/mtk_vcodec_util.c     |  10 -
 .../platform/mtk-vcodec/mtk_vcodec_util.h     |  45 +---
 .../mtk-vcodec/vdec/vdec_h264_req_if.c        |   4 +-
 .../platform/mtk-vcodec/venc/venc_h264_if.c   |   9 +-
 .../platform/mtk-vcodec/venc/venc_vp8_if.c    |   3 +-
 .../media/platform/mtk-vcodec/venc_vpu_if.c   |   1 -
 .../media/platform/mtk-vcodec/venc_vpu_if.h   |   1 -
 11 files changed, 118 insertions(+), 173 deletions(-)

-- 
2.17.1

[PATCH v2 1/7] media: mtk-vcodec: enc: add vp8 profile ctrl

[Dafna Hirschfeld]

In order for the encoder to work with gstreamer
it needs to have the V4L2_CID_MPEG_VIDEO_VP8_PROFILE
ctrl. This patch adds that ctrl with only profile 0
supported.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index 8998244ea671..87a5114bf680 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -103,6 +103,13 @@ static int vidioc_venc_s_ctrl(struct v4l2_ctrl *ctrl)
 		p->gop_size = ctrl->val;
 		ctx->param_change |= MTK_ENCODE_PARAM_GOP_SIZE;
 		break;
+	case V4L2_CID_MPEG_VIDEO_VP8_PROFILE:
+		/*
+		 * FIXME - what vp8 profiles are actually supported?
+		 * The ctrl is added (with only profile 0 supported) for now.
+		 */
+		mtk_v4l2_debug(2, "V4L2_CID_MPEG_VIDEO_VP8_PROFILE val = %d", ctrl->val);
+		break;
 	case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
 		mtk_v4l2_debug(2, "V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME");
 		p->force_intra = 1;
@@ -1394,6 +1401,9 @@ int mtk_vcodec_enc_ctrls_setup(struct mtk_vcodec_ctx *ctx)
 	v4l2_ctrl_new_std_menu(handler, ops, V4L2_CID_MPEG_VIDEO_H264_LEVEL,
 			       h264_max_level,
 			       0, V4L2_MPEG_VIDEO_H264_LEVEL_4_0);
+	v4l2_ctrl_new_std_menu(handler, ops, V4L2_CID_MPEG_VIDEO_VP8_PROFILE,
+			       V4L2_MPEG_VIDEO_VP8_PROFILE_0, 0, V4L2_MPEG_VIDEO_VP8_PROFILE_0);
+
 
 	if (handler->error) {
 		mtk_v4l2_err("Init control handler fail %d",
-- 
2.17.1

[PATCH v2 2/7] media: mtk-vcodec: call v4l2_m2m_ctx_release first when file is released

[Dafna Hirschfeld]

The func v4l2_m2m_ctx_release waits for currently running jobs
to finish and then stop streaming both queues and frees the buffers.
All this should be done before the call to mtk_vcodec_enc_release
which frees the encoder handler. This fixes null-pointer dereference bug:

[gst-master] root@debian:~/gst-build# [  638.019193] Unable to handle kernel NULL pointer dereference at virtual address 00000000000001a0
[  638.028076] Mem abort info:
[  638.030932]   ESR = 0x96000004
[  638.033978]   EC = 0x25: DABT (current EL), IL = 32 bits
[  638.039293]   SET = 0, FnV = 0
[  638.042338]   EA = 0, S1PTW = 0
[  638.045474]   FSC = 0x04: level 0 translation fault
[  638.050349] Data abort info:
[  638.053224]   ISV = 0, ISS = 0x00000004
[  638.057055]   CM = 0, WnR = 0
[  638.060018] user pgtable: 4k pages, 48-bit VAs, pgdp=000000012b6db000
[  638.066485] [00000000000001a0] pgd=0000000000000000, p4d=0000000000000000
[  638.073277] Internal error: Oops: 96000004 [#1] SMP
[  638.078145] Modules linked in: rfkill mtk_vcodec_dec mtk_vcodec_enc uvcvideo mtk_mdp mtk_vcodec_common videobuf2_dma_contig v4l2_h264 cdc_ether v4l2_mem2mem videobuf2_vmalloc usbnet videobuf2_memops videobuf2_v4l2 r8152 videobuf2_common videodev cros_ec_sensors cros_ec_sensors_core industrialio_triggered_buffer kfifo_buf elan_i2c elants_i2c sbs_battery mc cros_usbpd_charger cros_ec_chardev cros_usbpd_logger crct10dif_ce mtk_vpu fuse ip_tables x_tables ipv6
[  638.118583] CPU: 0 PID: 212 Comm: kworker/u8:5 Not tainted 5.15.0-06427-g58a1d4dcfc74-dirty #109
[  638.127357] Hardware name: Google Elm (DT)
[  638.131444] Workqueue: mtk-vcodec-enc mtk_venc_worker [mtk_vcodec_enc]
[  638.137974] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  638.144925] pc : vp8_enc_encode+0x34/0x2b0 [mtk_vcodec_enc]
[  638.150493] lr : venc_if_encode+0xac/0x1b0 [mtk_vcodec_enc]
[  638.156060] sp : ffff8000124d3c40
[  638.159364] x29: ffff8000124d3c40 x28: 0000000000000000 x27: 0000000000000000
[  638.166493] x26: 0000000000000000 x25: ffff0000e7f252d0 x24: ffff8000124d3d58
[  638.173621] x23: ffff8000124d3d58 x22: ffff8000124d3d60 x21: 0000000000000001
[  638.180750] x20: ffff80001137e000 x19: 0000000000000000 x18: 0000000000000001
[  638.187878] x17: 000000040044ffff x16: 00400032b5503510 x15: 0000000000000000
[  638.195006] x14: ffff8000118536c0 x13: ffff8000ee1da000 x12: 0000000030d4d91d
[  638.202134] x11: 0000000000000000 x10: 0000000000000980 x9 : ffff8000124d3b20
[  638.209262] x8 : ffff0000c18d4ea0 x7 : ffff0000c18d44c0 x6 : ffff0000c18d44c0
[  638.216391] x5 : ffff80000904a3b0 x4 : ffff8000124d3d58 x3 : ffff8000124d3d60
[  638.223519] x2 : ffff8000124d3d78 x1 : 0000000000000001 x0 : ffff80001137efb8
[  638.230648] Call trace:
[  638.233084]  vp8_enc_encode+0x34/0x2b0 [mtk_vcodec_enc]
[  638.238304]  venc_if_encode+0xac/0x1b0 [mtk_vcodec_enc]
[  638.243525]  mtk_venc_worker+0x110/0x250 [mtk_vcodec_enc]
[  638.248918]  process_one_work+0x1f8/0x498
[  638.252923]  worker_thread+0x140/0x538
[  638.256664]  kthread+0x148/0x158
[  638.259884]  ret_from_fork+0x10/0x20
[  638.263455] Code: f90023f9 2a0103f5 aa0303f6 aa0403f8 (f940d277)
[  638.269538] ---[ end trace e374fc10f8e181f5 ]---

Fixes: 4e855a6efa547 ("[media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver")
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
index eed67394cf46..f898226fc53e 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
@@ -214,11 +214,11 @@ static int fops_vcodec_release(struct file *file)
 	mtk_v4l2_debug(1, "[%d] encoder", ctx->id);
 	mutex_lock(&dev->dev_mutex);
 
+	v4l2_m2m_ctx_release(ctx->m2m_ctx);
 	mtk_vcodec_enc_release(ctx);
 	v4l2_fh_del(&ctx->fh);
 	v4l2_fh_exit(&ctx->fh);
 	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
-	v4l2_m2m_ctx_release(ctx->m2m_ctx);
 
 	list_del_init(&ctx->list);
 	kfree(ctx);
-- 
2.17.1

Re: [PATCH v2 2/7] media: mtk-vcodec: call v4l2_m2m_ctx_release first when file is released

[Alexandre Courbot]

On Wed, Nov 17, 2021 at 10:06 PM Dafna Hirschfeld
<dafna.hirschfeld@collabora.com> wrote:
>
> The func v4l2_m2m_ctx_release waits for currently running jobs
> to finish and then stop streaming both queues and frees the buffers.
> All this should be done before the call to mtk_vcodec_enc_release
> which frees the encoder handler. This fixes null-pointer dereference bug:
>
> [gst-master] root@debian:~/gst-build# [  638.019193] Unable to handle kernel NULL pointer dereference at virtual address 00000000000001a0
> [  638.028076] Mem abort info:
> [  638.030932]   ESR = 0x96000004
> [  638.033978]   EC = 0x25: DABT (current EL), IL = 32 bits
> [  638.039293]   SET = 0, FnV = 0
> [  638.042338]   EA = 0, S1PTW = 0
> [  638.045474]   FSC = 0x04: level 0 translation fault
> [  638.050349] Data abort info:
> [  638.053224]   ISV = 0, ISS = 0x00000004
> [  638.057055]   CM = 0, WnR = 0
> [  638.060018] user pgtable: 4k pages, 48-bit VAs, pgdp=000000012b6db000
> [  638.066485] [00000000000001a0] pgd=0000000000000000, p4d=0000000000000000
> [  638.073277] Internal error: Oops: 96000004 [#1] SMP
> [  638.078145] Modules linked in: rfkill mtk_vcodec_dec mtk_vcodec_enc uvcvideo mtk_mdp mtk_vcodec_common videobuf2_dma_contig v4l2_h264 cdc_ether v4l2_mem2mem videobuf2_vmalloc usbnet videobuf2_memops videobuf2_v4l2 r8152 videobuf2_common videodev cros_ec_sensors cros_ec_sensors_core industrialio_triggered_buffer kfifo_buf elan_i2c elants_i2c sbs_battery mc cros_usbpd_charger cros_ec_chardev cros_usbpd_logger crct10dif_ce mtk_vpu fuse ip_tables x_tables ipv6
> [  638.118583] CPU: 0 PID: 212 Comm: kworker/u8:5 Not tainted 5.15.0-06427-g58a1d4dcfc74-dirty #109
> [  638.127357] Hardware name: Google Elm (DT)
> [  638.131444] Workqueue: mtk-vcodec-enc mtk_venc_worker [mtk_vcodec_enc]
> [  638.137974] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [  638.144925] pc : vp8_enc_encode+0x34/0x2b0 [mtk_vcodec_enc]
> [  638.150493] lr : venc_if_encode+0xac/0x1b0 [mtk_vcodec_enc]
> [  638.156060] sp : ffff8000124d3c40
> [  638.159364] x29: ffff8000124d3c40 x28: 0000000000000000 x27: 0000000000000000
> [  638.166493] x26: 0000000000000000 x25: ffff0000e7f252d0 x24: ffff8000124d3d58
> [  638.173621] x23: ffff8000124d3d58 x22: ffff8000124d3d60 x21: 0000000000000001
> [  638.180750] x20: ffff80001137e000 x19: 0000000000000000 x18: 0000000000000001
> [  638.187878] x17: 000000040044ffff x16: 00400032b5503510 x15: 0000000000000000
> [  638.195006] x14: ffff8000118536c0 x13: ffff8000ee1da000 x12: 0000000030d4d91d
> [  638.202134] x11: 0000000000000000 x10: 0000000000000980 x9 : ffff8000124d3b20
> [  638.209262] x8 : ffff0000c18d4ea0 x7 : ffff0000c18d44c0 x6 : ffff0000c18d44c0
> [  638.216391] x5 : ffff80000904a3b0 x4 : ffff8000124d3d58 x3 : ffff8000124d3d60
> [  638.223519] x2 : ffff8000124d3d78 x1 : 0000000000000001 x0 : ffff80001137efb8
> [  638.230648] Call trace:
> [  638.233084]  vp8_enc_encode+0x34/0x2b0 [mtk_vcodec_enc]
> [  638.238304]  venc_if_encode+0xac/0x1b0 [mtk_vcodec_enc]
> [  638.243525]  mtk_venc_worker+0x110/0x250 [mtk_vcodec_enc]
> [  638.248918]  process_one_work+0x1f8/0x498
> [  638.252923]  worker_thread+0x140/0x538
> [  638.256664]  kthread+0x148/0x158
> [  638.259884]  ret_from_fork+0x10/0x20
> [  638.263455] Code: f90023f9 2a0103f5 aa0303f6 aa0403f8 (f940d277)
> [  638.269538] ---[ end trace e374fc10f8e181f5 ]---
>
> Fixes: 4e855a6efa547 ("[media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver")
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

The order looks indeed safer this way.

Reviewed-by: Alexandre Courbot <acourbot@chromium.org>


> ---
>  drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> index eed67394cf46..f898226fc53e 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
> @@ -214,11 +214,11 @@ static int fops_vcodec_release(struct file *file)
>         mtk_v4l2_debug(1, "[%d] encoder", ctx->id);
>         mutex_lock(&dev->dev_mutex);
>
> +       v4l2_m2m_ctx_release(ctx->m2m_ctx);
>         mtk_vcodec_enc_release(ctx);
>         v4l2_fh_del(&ctx->fh);
>         v4l2_fh_exit(&ctx->fh);
>         v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> -       v4l2_m2m_ctx_release(ctx->m2m_ctx);
>
>         list_del_init(&ctx->list);
>         kfree(ctx);
> --
> 2.17.1
>

[PATCH v2 3/7] media: mtk-vcodec: enc: use "stream_started" flag for "stop/start_streaming"

[Dafna Hirschfeld]

Currently the mtk-vcodec encoder does runtime pm resume
in "start_streaming" cb if both queues are streaming
and does runtime pm 'put' in "stop_streaming" if both
queues are not streaming.
This is wrong since the same queue might be started and
then stopped causing the driver to turn off the hardware
without turning it on. This cause for example unbalanced
calls to pm_runtime_*

Fixes: 4e855a6efa547 ("[media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver")
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
to reproduce the issue:
patch v4l-utils as follow:

static void stateful_m2m(cv4l_fd &fd, cv4l_queue &in, cv4l_queue &out,

 	if (fd.streamon(out.g_type()))
 		return;
+	if (fd.streamoff(out.g_type()))
+		return;
+
+	exit(1);

 	fd.s_trace(0);
 	if (exp_fd_p)

and call:
v4l2-ctl -x width=160,height=128,pixelformat=NM12 -v pixelformat=VP80 --stream-mmap --stream-out-mmap -d5
then the file /sys/devices/platform/soc/19002000.vcodec/power/runtime_usage
will show a negative number and further streaming (with e.g, gstreamer) is not possible.

 drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 4 ++++
 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
index 9d36e3d27369..84c5289f872b 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
@@ -259,6 +259,9 @@ struct vdec_pic_info {
  * @decoded_frame_cnt: number of decoded frames
  * @lock: protect variables accessed by V4L2 threads and worker thread such as
  *	  mtk_video_dec_buf.
+ * @stream_started: this flag is turned on when both queues (cap and out) starts streaming
+ *	  and it is turned off once both queues stop streaming. It is used for a correct
+ *	  setup and set-down of the hardware when starting and stopping streaming.
  */
 struct mtk_vcodec_ctx {
 	enum mtk_instance_type type;
@@ -301,6 +304,7 @@ struct mtk_vcodec_ctx {
 
 	int decoded_frame_cnt;
 	struct mutex lock;
+	bool stream_started;
 
 };
 
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index 87a5114bf680..fb3cf804c96a 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -890,6 +890,9 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
 		goto err_start_stream;
 	}
 
+	if (ctx->stream_started)
+		return 0;
+
 	/* Do the initialization when both start_streaming have been called */
 	if (V4L2_TYPE_IS_OUTPUT(q->type)) {
 		if (!vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q))
@@ -928,6 +931,7 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
 		ctx->state = MTK_STATE_HEADER;
 	}
 
+	ctx->stream_started = true;
 	return 0;
 
 err_set_param:
@@ -1002,6 +1006,9 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
 		}
 	}
 
+	if (!ctx->stream_started)
+		return;
+
 	if ((q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
 	     vb2_is_streaming(&ctx->m2m_ctx->out_q_ctx.q)) ||
 	    (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
@@ -1023,6 +1030,7 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
 		mtk_v4l2_err("pm_runtime_put fail %d", ret);
 
 	ctx->state = MTK_STATE_FREE;
+	ctx->stream_started = false;
 }
 
 static int vb2ops_venc_buf_out_validate(struct vb2_buffer *vb)
-- 
2.17.1

Re: [PATCH v2 3/7] media: mtk-vcodec: enc: use "stream_started" flag for "stop/start_streaming"

[Hans Verkuil]

Hi Dafna,

On 17/11/2021 14:06, Dafna Hirschfeld wrote:
> Currently the mtk-vcodec encoder does runtime pm resume
> in "start_streaming" cb if both queues are streaming
> and does runtime pm 'put' in "stop_streaming" if both
> queues are not streaming.
> This is wrong since the same queue might be started and
> then stopped causing the driver to turn off the hardware
> without turning it on. This cause for example unbalanced
> calls to pm_runtime_*
> 
> Fixes: 4e855a6efa547 ("[media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver")
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> to reproduce the issue:
> patch v4l-utils as follow:
> 
> static void stateful_m2m(cv4l_fd &fd, cv4l_queue &in, cv4l_queue &out,
> 
>  	if (fd.streamon(out.g_type()))
>  		return;
> +	if (fd.streamoff(out.g_type()))
> +		return;
> +
> +	exit(1);
> 
>  	fd.s_trace(0);
>  	if (exp_fd_p)
> 
> and call:
> v4l2-ctl -x width=160,height=128,pixelformat=NM12 -v pixelformat=VP80 --stream-mmap --stream-out-mmap -d5
> then the file /sys/devices/platform/soc/19002000.vcodec/power/runtime_usage
> will show a negative number and further streaming (with e.g, gstreamer) is not possible.
> 
>  drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 4 ++++
>  drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 8 ++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> index 9d36e3d27369..84c5289f872b 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
> @@ -259,6 +259,9 @@ struct vdec_pic_info {
>   * @decoded_frame_cnt: number of decoded frames
>   * @lock: protect variables accessed by V4L2 threads and worker thread such as
>   *	  mtk_video_dec_buf.
> + * @stream_started: this flag is turned on when both queues (cap and out) starts streaming
> + *	  and it is turned off once both queues stop streaming. It is used for a correct
> + *	  setup and set-down of the hardware when starting and stopping streaming.
>   */
>  struct mtk_vcodec_ctx {
>  	enum mtk_instance_type type;
> @@ -301,6 +304,7 @@ struct mtk_vcodec_ctx {
>  
>  	int decoded_frame_cnt;
>  	struct mutex lock;
> +	bool stream_started;
>  
>  };
>  
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> index 87a5114bf680..fb3cf804c96a 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> @@ -890,6 +890,9 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
>  		goto err_start_stream;
>  	}
>  
> +	if (ctx->stream_started)
> +		return 0;
> +
>  	/* Do the initialization when both start_streaming have been called */
>  	if (V4L2_TYPE_IS_OUTPUT(q->type)) {
>  		if (!vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q))
> @@ -928,6 +931,7 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
>  		ctx->state = MTK_STATE_HEADER;
>  	}
>  
> +	ctx->stream_started = true;
>  	return 0;
>  
>  err_set_param:
> @@ -1002,6 +1006,9 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
>  		}
>  	}
>  
> +	if (!ctx->stream_started)
> +		return;
> +
>  	if ((q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
>  	     vb2_is_streaming(&ctx->m2m_ctx->out_q_ctx.q)) ||
>  	    (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&

(copy-and-pasted from my reply to the same patch in the v1 series, you probably
missed that reply...)

I don't think this patch is the right fix. I think the real problem is that
vb2ops_venc_start_streaming() calls vb2_start_streaming_called() to
check that the other queue is also ready to start streaming, whereas
vb2ops_venc_stop_streaming() incorrectly calls vb2_is_streaming()
instead of vb2_start_streaming_called().

So my guess is that this mismatch is what causes the problem. Regardless,
it is definitely a bug that vb2ops_venc_stop_streaming() calls vb2_is_streaming().

I'm marking this patch as 'Changes Requested', but I'll accept the other patches
this series (with patch 4/7 fixed to take care of the kernel test robot report).

Regards,

	Hans

> @@ -1023,6 +1030,7 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
>  		mtk_v4l2_err("pm_runtime_put fail %d", ret);
>  
>  	ctx->state = MTK_STATE_FREE;
> +	ctx->stream_started = false;
>  }
>  
>  static int vb2ops_venc_buf_out_validate(struct vb2_buffer *vb)
> 

[PATCH v2 4/7] media: mtk-vcodec: fix debugging defines

[Dafna Hirschfeld]

The mtk-vcodec uses some internal defined debug formats for
printing. This patch fixes some things in those defines:

1. use the 'pr_fmt' define to print function name and line.

2. remove 'if(DEBUG)' condition for the defines. This condition
prevents the debugs from being shown in case of dynamic debugs.
Instead replace 'pr_info' with 'pr_debug'

3. remove module parameters that enable/disable debug.
There is no reason for the driver to have those params. Having
those params require the user to explicitly set them when user
wants to see debug prints instead of using the global debugs
setting as expected by drivers to conform.

In addition to that, fix some warnings about debug formatting

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c  |  3 --
 .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c  |  3 --
 .../platform/mtk-vcodec/mtk_vcodec_util.c     | 10 -----
 .../platform/mtk-vcodec/mtk_vcodec_util.h     | 45 +++++--------------
 .../mtk-vcodec/vdec/vdec_h264_req_if.c        |  4 +-
 5 files changed, 12 insertions(+), 53 deletions(-)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c
index e6e6a8203eeb..f3610a338a01 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c
@@ -28,9 +28,6 @@
 #define VDEC_IRQ_CLR	0x10
 #define VDEC_IRQ_CFG_REG	0xa4
 
-module_param(mtk_v4l2_dbg_level, int, 0644);
-module_param(mtk_vcodec_dbg, bool, 0644);
-
 /* Wake up context wait_queue */
 static void wake_up_ctx(struct mtk_vcodec_ctx *ctx)
 {
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
index f898226fc53e..ec5ee337c1fd 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_drv.c
@@ -23,9 +23,6 @@
 #include "mtk_vcodec_util.h"
 #include "mtk_vcodec_fw.h"
 
-module_param(mtk_v4l2_dbg_level, int, S_IRUGO | S_IWUSR);
-module_param(mtk_vcodec_dbg, bool, S_IRUGO | S_IWUSR);
-
 static const struct mtk_video_fmt mtk_video_formats_output[] = {
 	{
 		.fourcc = V4L2_PIX_FMT_NV12M,
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.c
index ac5973b6735f..5bac820a47fc 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.c
@@ -10,16 +10,6 @@
 #include "mtk_vcodec_drv.h"
 #include "mtk_vcodec_util.h"
 
-/* For encoder, this will enable logs in venc/*/
-bool mtk_vcodec_dbg;
-EXPORT_SYMBOL(mtk_vcodec_dbg);
-
-/* The log level of v4l2 encoder or decoder driver.
- * That is, files under mtk-vcodec/.
- */
-int mtk_v4l2_dbg_level;
-EXPORT_SYMBOL(mtk_v4l2_dbg_level);
-
 void __iomem *mtk_vcodec_get_reg_addr(struct mtk_vcodec_ctx *data,
 					unsigned int reg_idx)
 {
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
index b999d7b84ed1..87c3d6d4bfa7 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h
@@ -25,54 +25,29 @@ struct mtk_vcodec_fb {
 struct mtk_vcodec_ctx;
 struct mtk_vcodec_dev;
 
-extern int mtk_v4l2_dbg_level;
-extern bool mtk_vcodec_dbg;
-
+#undef pr_fmt
+#define pr_fmt(fmt) "%s(),%d: " fmt, __func__, __LINE__
 
 #define mtk_v4l2_err(fmt, args...)                \
-	pr_err("[MTK_V4L2][ERROR] %s:%d: " fmt "\n", __func__, __LINE__, \
-	       ##args)
-
-#define mtk_vcodec_err(h, fmt, args...)					\
-	pr_err("[MTK_VCODEC][ERROR][%d]: %s() " fmt "\n",		\
-	       ((struct mtk_vcodec_ctx *)h->ctx)->id, __func__, ##args)
+	pr_err("[MTK_V4L2][ERROR] " fmt "\n", ##args)
 
+#define mtk_vcodec_err(h, fmt, args...)				\
+	pr_err("[MTK_VCODEC][ERROR][%d]: " fmt "\n",		\
+	       ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
 
-#if defined(DEBUG)
 
-#define mtk_v4l2_debug(level, fmt, args...)				 \
-	do {								 \
-		if (mtk_v4l2_dbg_level >= level)			 \
-			pr_info("[MTK_V4L2] level=%d %s(),%d: " fmt "\n",\
-				level, __func__, __LINE__, ##args);	 \
-	} while (0)
+#define mtk_v4l2_debug(level, fmt, args...) pr_debug(fmt, ##args)
 
 #define mtk_v4l2_debug_enter()  mtk_v4l2_debug(3, "+")
 #define mtk_v4l2_debug_leave()  mtk_v4l2_debug(3, "-")
 
-#define mtk_vcodec_debug(h, fmt, args...)				\
-	do {								\
-		if (mtk_vcodec_dbg)					\
-			pr_info("[MTK_VCODEC][%d]: %s() " fmt "\n",	\
-				((struct mtk_vcodec_ctx *)h->ctx)->id, \
-				__func__, ##args);			\
-	} while (0)
+#define mtk_vcodec_debug(h, fmt, args...)			\
+	pr_debug("[MTK_VCODEC][%d]: " fmt "\n",			\
+		((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
 
 #define mtk_vcodec_debug_enter(h)  mtk_vcodec_debug(h, "+")
 #define mtk_vcodec_debug_leave(h)  mtk_vcodec_debug(h, "-")
 
-#else
-
-#define mtk_v4l2_debug(level, fmt, args...) {}
-#define mtk_v4l2_debug_enter() {}
-#define mtk_v4l2_debug_leave() {}
-
-#define mtk_vcodec_debug(h, fmt, args...) {}
-#define mtk_vcodec_debug_enter(h) {}
-#define mtk_vcodec_debug_leave(h) {}
-
-#endif
-
 void __iomem *mtk_vcodec_get_reg_addr(struct mtk_vcodec_ctx *data,
 				unsigned int reg_idx);
 int mtk_vcodec_mem_alloc(struct mtk_vcodec_ctx *data,
diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
index 946c23088308..88f2e8f9bfe1 100644
--- a/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
+++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c
@@ -527,7 +527,7 @@ static int alloc_mv_buf(struct vdec_h264_slice_inst *inst,
 	struct mtk_vcodec_mem *mem = NULL;
 	unsigned int buf_sz = get_mv_buf_size(pic->buf_w, pic->buf_h);
 
-	mtk_v4l2_debug(3, "size = 0x%lx", buf_sz);
+	mtk_v4l2_debug(3, "size = 0x%x", buf_sz);
 	for (i = 0; i < H264_MAX_MV_NUM; i++) {
 		mem = &inst->mv_buf[i];
 		if (mem->va)
@@ -637,7 +637,7 @@ static int vdec_h264_slice_init(struct mtk_vcodec_ctx *ctx)
 	if (err)
 		goto error_deinit;
 
-	mtk_vcodec_debug(inst, "struct size = %d,%d,%d,%d\n",
+	mtk_vcodec_debug(inst, "struct size = %lu,%lu,%lu,%lu\n",
 			 sizeof(struct mtk_h264_sps_param),
 			 sizeof(struct mtk_h264_pps_param),
 			 sizeof(struct mtk_h264_dec_slice_param),
-- 
2.17.1

[PATCH v2 5/7] media: mtk-vcodec: replace func vidioc_try_fmt with two funcs for out/cap

[Dafna Hirschfeld]

The function vidioc_try_fmt has a big 'if-else' for
the capture and output cases. There is hardly any code
outside of that condition. It is therefore better to split
that functions into two different functions, one for each case.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 .../platform/mtk-vcodec/mtk_vcodec_enc.c      | 149 +++++++++---------
 1 file changed, 72 insertions(+), 77 deletions(-)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index fb3cf804c96a..be28f2401839 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -263,87 +263,82 @@ static struct mtk_q_data *mtk_venc_get_q_data(struct mtk_vcodec_ctx *ctx,
 	return &ctx->q_data[MTK_Q_DATA_DST];
 }
 
+static void vidioc_try_fmt_cap(struct v4l2_format *f)
+{
+	f->fmt.pix_mp.field = V4L2_FIELD_NONE;
+	f->fmt.pix_mp.num_planes = 1;
+	f->fmt.pix_mp.plane_fmt[0].bytesperline = 0;
+	f->fmt.pix_mp.flags = 0;
+}
+
 /* V4L2 specification suggests the driver corrects the format struct if any of
  * the dimensions is unsupported
  */
-static int vidioc_try_fmt(struct mtk_vcodec_ctx *ctx, struct v4l2_format *f,
-			  const struct mtk_video_fmt *fmt)
+static int vidioc_try_fmt_out(struct mtk_vcodec_ctx *ctx, struct v4l2_format *f,
+			      const struct mtk_video_fmt *fmt)
 {
 	struct v4l2_pix_format_mplane *pix_fmt_mp = &f->fmt.pix_mp;
+	int tmp_w, tmp_h;
+	unsigned int max_width, max_height;
 
 	pix_fmt_mp->field = V4L2_FIELD_NONE;
 
-	if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
-		pix_fmt_mp->num_planes = 1;
-		pix_fmt_mp->plane_fmt[0].bytesperline = 0;
-	} else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
-		int tmp_w, tmp_h;
-		unsigned int max_width, max_height;
-
-		if (ctx->dev->enc_capability & MTK_VENC_4K_CAPABILITY_ENABLE) {
-			max_width = MTK_VENC_4K_MAX_W;
-			max_height = MTK_VENC_4K_MAX_H;
-		} else {
-			max_width = MTK_VENC_HD_MAX_W;
-			max_height = MTK_VENC_HD_MAX_H;
-		}
+	if (ctx->dev->enc_capability & MTK_VENC_4K_CAPABILITY_ENABLE) {
+		max_width = MTK_VENC_4K_MAX_W;
+		max_height = MTK_VENC_4K_MAX_H;
+	} else {
+		max_width = MTK_VENC_HD_MAX_W;
+		max_height = MTK_VENC_HD_MAX_H;
+	}
 
-		pix_fmt_mp->height = clamp(pix_fmt_mp->height,
-					MTK_VENC_MIN_H,
-					max_height);
-		pix_fmt_mp->width = clamp(pix_fmt_mp->width,
-					MTK_VENC_MIN_W,
-					max_width);
+	pix_fmt_mp->height = clamp(pix_fmt_mp->height, MTK_VENC_MIN_H, max_height);
+	pix_fmt_mp->width = clamp(pix_fmt_mp->width, MTK_VENC_MIN_W, max_width);
 
-		/* find next closer width align 16, heign align 32, size align
-		 * 64 rectangle
-		 */
-		tmp_w = pix_fmt_mp->width;
-		tmp_h = pix_fmt_mp->height;
-		v4l_bound_align_image(&pix_fmt_mp->width,
-					MTK_VENC_MIN_W,
-					max_width, 4,
-					&pix_fmt_mp->height,
-					MTK_VENC_MIN_H,
-					max_height, 5, 6);
-
-		if (pix_fmt_mp->width < tmp_w &&
-			(pix_fmt_mp->width + 16) <= max_width)
-			pix_fmt_mp->width += 16;
-		if (pix_fmt_mp->height < tmp_h &&
-			(pix_fmt_mp->height + 32) <= max_height)
-			pix_fmt_mp->height += 32;
-
-		mtk_v4l2_debug(0,
-			"before resize width=%d, height=%d, after resize width=%d, height=%d, sizeimage=%d %d",
-			tmp_w, tmp_h, pix_fmt_mp->width,
-			pix_fmt_mp->height,
-			pix_fmt_mp->plane_fmt[0].sizeimage,
-			pix_fmt_mp->plane_fmt[1].sizeimage);
-
-		pix_fmt_mp->num_planes = fmt->num_planes;
-		pix_fmt_mp->plane_fmt[0].sizeimage =
-				pix_fmt_mp->width * pix_fmt_mp->height +
-				((ALIGN(pix_fmt_mp->width, 16) * 2) * 16);
-		pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
-
-		if (pix_fmt_mp->num_planes == 2) {
-			pix_fmt_mp->plane_fmt[1].sizeimage =
-				(pix_fmt_mp->width * pix_fmt_mp->height) / 2 +
-				(ALIGN(pix_fmt_mp->width, 16) * 16);
-			pix_fmt_mp->plane_fmt[2].sizeimage = 0;
-			pix_fmt_mp->plane_fmt[1].bytesperline =
-							pix_fmt_mp->width;
-			pix_fmt_mp->plane_fmt[2].bytesperline = 0;
-		} else if (pix_fmt_mp->num_planes == 3) {
-			pix_fmt_mp->plane_fmt[1].sizeimage =
-			pix_fmt_mp->plane_fmt[2].sizeimage =
-				(pix_fmt_mp->width * pix_fmt_mp->height) / 4 +
-				((ALIGN(pix_fmt_mp->width, 16) / 2) * 16);
-			pix_fmt_mp->plane_fmt[1].bytesperline =
-				pix_fmt_mp->plane_fmt[2].bytesperline =
-				pix_fmt_mp->width / 2;
-		}
+	/* find next closer width align 16, heign align 32, size align
+	 * 64 rectangle
+	 */
+	tmp_w = pix_fmt_mp->width;
+	tmp_h = pix_fmt_mp->height;
+	v4l_bound_align_image(&pix_fmt_mp->width,
+			      MTK_VENC_MIN_W,
+			      max_width, 4,
+			      &pix_fmt_mp->height,
+			      MTK_VENC_MIN_H,
+			      max_height, 5, 6);
+
+	if (pix_fmt_mp->width < tmp_w && (pix_fmt_mp->width + 16) <= max_width)
+		pix_fmt_mp->width += 16;
+	if (pix_fmt_mp->height < tmp_h && (pix_fmt_mp->height + 32) <= max_height)
+		pix_fmt_mp->height += 32;
+
+	mtk_v4l2_debug(0, "before resize w=%d, h=%d, after resize w=%d, h=%d, sizeimage=%d %d",
+		       tmp_w, tmp_h, pix_fmt_mp->width,
+		       pix_fmt_mp->height,
+		       pix_fmt_mp->plane_fmt[0].sizeimage,
+		       pix_fmt_mp->plane_fmt[1].sizeimage);
+
+	pix_fmt_mp->num_planes = fmt->num_planes;
+	pix_fmt_mp->plane_fmt[0].sizeimage =
+			pix_fmt_mp->width * pix_fmt_mp->height +
+			((ALIGN(pix_fmt_mp->width, 16) * 2) * 16);
+	pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
+
+	if (pix_fmt_mp->num_planes == 2) {
+		pix_fmt_mp->plane_fmt[1].sizeimage =
+			(pix_fmt_mp->width * pix_fmt_mp->height) / 2 +
+			(ALIGN(pix_fmt_mp->width, 16) * 16);
+		pix_fmt_mp->plane_fmt[2].sizeimage = 0;
+		pix_fmt_mp->plane_fmt[1].bytesperline =
+						pix_fmt_mp->width;
+		pix_fmt_mp->plane_fmt[2].bytesperline = 0;
+	} else if (pix_fmt_mp->num_planes == 3) {
+		pix_fmt_mp->plane_fmt[1].sizeimage =
+		pix_fmt_mp->plane_fmt[2].sizeimage =
+			(pix_fmt_mp->width * pix_fmt_mp->height) / 4 +
+			((ALIGN(pix_fmt_mp->width, 16) / 2) * 16);
+		pix_fmt_mp->plane_fmt[1].bytesperline =
+			pix_fmt_mp->plane_fmt[2].bytesperline =
+			pix_fmt_mp->width / 2;
 	}
 
 	pix_fmt_mp->flags = 0;
@@ -432,9 +427,7 @@ static int vidioc_venc_s_fmt_cap(struct file *file, void *priv,
 	}
 
 	q_data->fmt = fmt;
-	ret = vidioc_try_fmt(ctx, f, q_data->fmt);
-	if (ret)
-		return ret;
+	vidioc_try_fmt_cap(f);
 
 	q_data->coded_width = f->fmt.pix_mp.width;
 	q_data->coded_height = f->fmt.pix_mp.height;
@@ -494,7 +487,7 @@ static int vidioc_venc_s_fmt_out(struct file *file, void *priv,
 		f->fmt.pix.pixelformat = fmt->fourcc;
 	}
 
-	ret = vidioc_try_fmt(ctx, f, fmt);
+	ret = vidioc_try_fmt_out(ctx, f, fmt);
 	if (ret)
 		return ret;
 
@@ -572,7 +565,9 @@ static int vidioc_try_fmt_vid_cap_mplane(struct file *file, void *priv,
 	f->fmt.pix_mp.quantization = ctx->quantization;
 	f->fmt.pix_mp.xfer_func = ctx->xfer_func;
 
-	return vidioc_try_fmt(ctx, f, fmt);
+	vidioc_try_fmt_cap(f);
+
+	return 0;
 }
 
 static int vidioc_try_fmt_vid_out_mplane(struct file *file, void *priv,
@@ -594,7 +589,7 @@ static int vidioc_try_fmt_vid_out_mplane(struct file *file, void *priv,
 		f->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT;
 	}
 
-	return vidioc_try_fmt(ctx, f, fmt);
+	return vidioc_try_fmt_out(ctx, f, fmt);
 }
 
 static int vidioc_venc_g_selection(struct file *file, void *priv,
-- 
2.17.1

Re: [PATCH v2 5/7] media: mtk-vcodec: replace func vidioc_try_fmt with two funcs for out/cap

[Alexandre Courbot]

On Wed, Nov 17, 2021 at 10:07 PM Dafna Hirschfeld
<dafna.hirschfeld@collabora.com> wrote:
>
> The function vidioc_try_fmt has a big 'if-else' for
> the capture and output cases. There is hardly any code
> outside of that condition. It is therefore better to split
> that functions into two different functions, one for each case.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

Makes much more sense that way, thanks!

Reviewed-by: Alexandre Courbot <acourbot@chromium.org>


> ---
>  .../platform/mtk-vcodec/mtk_vcodec_enc.c      | 149 +++++++++---------
>  1 file changed, 72 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> index fb3cf804c96a..be28f2401839 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> @@ -263,87 +263,82 @@ static struct mtk_q_data *mtk_venc_get_q_data(struct mtk_vcodec_ctx *ctx,
>         return &ctx->q_data[MTK_Q_DATA_DST];
>  }
>
> +static void vidioc_try_fmt_cap(struct v4l2_format *f)
> +{
> +       f->fmt.pix_mp.field = V4L2_FIELD_NONE;
> +       f->fmt.pix_mp.num_planes = 1;
> +       f->fmt.pix_mp.plane_fmt[0].bytesperline = 0;
> +       f->fmt.pix_mp.flags = 0;
> +}
> +
>  /* V4L2 specification suggests the driver corrects the format struct if any of
>   * the dimensions is unsupported
>   */
> -static int vidioc_try_fmt(struct mtk_vcodec_ctx *ctx, struct v4l2_format *f,
> -                         const struct mtk_video_fmt *fmt)
> +static int vidioc_try_fmt_out(struct mtk_vcodec_ctx *ctx, struct v4l2_format *f,
> +                             const struct mtk_video_fmt *fmt)
>  {
>         struct v4l2_pix_format_mplane *pix_fmt_mp = &f->fmt.pix_mp;
> +       int tmp_w, tmp_h;
> +       unsigned int max_width, max_height;
>
>         pix_fmt_mp->field = V4L2_FIELD_NONE;
>
> -       if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> -               pix_fmt_mp->num_planes = 1;
> -               pix_fmt_mp->plane_fmt[0].bytesperline = 0;
> -       } else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> -               int tmp_w, tmp_h;
> -               unsigned int max_width, max_height;
> -
> -               if (ctx->dev->enc_capability & MTK_VENC_4K_CAPABILITY_ENABLE) {
> -                       max_width = MTK_VENC_4K_MAX_W;
> -                       max_height = MTK_VENC_4K_MAX_H;
> -               } else {
> -                       max_width = MTK_VENC_HD_MAX_W;
> -                       max_height = MTK_VENC_HD_MAX_H;
> -               }
> +       if (ctx->dev->enc_capability & MTK_VENC_4K_CAPABILITY_ENABLE) {
> +               max_width = MTK_VENC_4K_MAX_W;
> +               max_height = MTK_VENC_4K_MAX_H;
> +       } else {
> +               max_width = MTK_VENC_HD_MAX_W;
> +               max_height = MTK_VENC_HD_MAX_H;
> +       }
>
> -               pix_fmt_mp->height = clamp(pix_fmt_mp->height,
> -                                       MTK_VENC_MIN_H,
> -                                       max_height);
> -               pix_fmt_mp->width = clamp(pix_fmt_mp->width,
> -                                       MTK_VENC_MIN_W,
> -                                       max_width);
> +       pix_fmt_mp->height = clamp(pix_fmt_mp->height, MTK_VENC_MIN_H, max_height);
> +       pix_fmt_mp->width = clamp(pix_fmt_mp->width, MTK_VENC_MIN_W, max_width);
>
> -               /* find next closer width align 16, heign align 32, size align
> -                * 64 rectangle
> -                */
> -               tmp_w = pix_fmt_mp->width;
> -               tmp_h = pix_fmt_mp->height;
> -               v4l_bound_align_image(&pix_fmt_mp->width,
> -                                       MTK_VENC_MIN_W,
> -                                       max_width, 4,
> -                                       &pix_fmt_mp->height,
> -                                       MTK_VENC_MIN_H,
> -                                       max_height, 5, 6);
> -
> -               if (pix_fmt_mp->width < tmp_w &&
> -                       (pix_fmt_mp->width + 16) <= max_width)
> -                       pix_fmt_mp->width += 16;
> -               if (pix_fmt_mp->height < tmp_h &&
> -                       (pix_fmt_mp->height + 32) <= max_height)
> -                       pix_fmt_mp->height += 32;
> -
> -               mtk_v4l2_debug(0,
> -                       "before resize width=%d, height=%d, after resize width=%d, height=%d, sizeimage=%d %d",
> -                       tmp_w, tmp_h, pix_fmt_mp->width,
> -                       pix_fmt_mp->height,
> -                       pix_fmt_mp->plane_fmt[0].sizeimage,
> -                       pix_fmt_mp->plane_fmt[1].sizeimage);
> -
> -               pix_fmt_mp->num_planes = fmt->num_planes;
> -               pix_fmt_mp->plane_fmt[0].sizeimage =
> -                               pix_fmt_mp->width * pix_fmt_mp->height +
> -                               ((ALIGN(pix_fmt_mp->width, 16) * 2) * 16);
> -               pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
> -
> -               if (pix_fmt_mp->num_planes == 2) {
> -                       pix_fmt_mp->plane_fmt[1].sizeimage =
> -                               (pix_fmt_mp->width * pix_fmt_mp->height) / 2 +
> -                               (ALIGN(pix_fmt_mp->width, 16) * 16);
> -                       pix_fmt_mp->plane_fmt[2].sizeimage = 0;
> -                       pix_fmt_mp->plane_fmt[1].bytesperline =
> -                                                       pix_fmt_mp->width;
> -                       pix_fmt_mp->plane_fmt[2].bytesperline = 0;
> -               } else if (pix_fmt_mp->num_planes == 3) {
> -                       pix_fmt_mp->plane_fmt[1].sizeimage =
> -                       pix_fmt_mp->plane_fmt[2].sizeimage =
> -                               (pix_fmt_mp->width * pix_fmt_mp->height) / 4 +
> -                               ((ALIGN(pix_fmt_mp->width, 16) / 2) * 16);
> -                       pix_fmt_mp->plane_fmt[1].bytesperline =
> -                               pix_fmt_mp->plane_fmt[2].bytesperline =
> -                               pix_fmt_mp->width / 2;
> -               }
> +       /* find next closer width align 16, heign align 32, size align
> +        * 64 rectangle
> +        */
> +       tmp_w = pix_fmt_mp->width;
> +       tmp_h = pix_fmt_mp->height;
> +       v4l_bound_align_image(&pix_fmt_mp->width,
> +                             MTK_VENC_MIN_W,
> +                             max_width, 4,
> +                             &pix_fmt_mp->height,
> +                             MTK_VENC_MIN_H,
> +                             max_height, 5, 6);
> +
> +       if (pix_fmt_mp->width < tmp_w && (pix_fmt_mp->width + 16) <= max_width)
> +               pix_fmt_mp->width += 16;
> +       if (pix_fmt_mp->height < tmp_h && (pix_fmt_mp->height + 32) <= max_height)
> +               pix_fmt_mp->height += 32;
> +
> +       mtk_v4l2_debug(0, "before resize w=%d, h=%d, after resize w=%d, h=%d, sizeimage=%d %d",
> +                      tmp_w, tmp_h, pix_fmt_mp->width,
> +                      pix_fmt_mp->height,
> +                      pix_fmt_mp->plane_fmt[0].sizeimage,
> +                      pix_fmt_mp->plane_fmt[1].sizeimage);
> +
> +       pix_fmt_mp->num_planes = fmt->num_planes;
> +       pix_fmt_mp->plane_fmt[0].sizeimage =
> +                       pix_fmt_mp->width * pix_fmt_mp->height +
> +                       ((ALIGN(pix_fmt_mp->width, 16) * 2) * 16);
> +       pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
> +
> +       if (pix_fmt_mp->num_planes == 2) {
> +               pix_fmt_mp->plane_fmt[1].sizeimage =
> +                       (pix_fmt_mp->width * pix_fmt_mp->height) / 2 +
> +                       (ALIGN(pix_fmt_mp->width, 16) * 16);
> +               pix_fmt_mp->plane_fmt[2].sizeimage = 0;
> +               pix_fmt_mp->plane_fmt[1].bytesperline =
> +                                               pix_fmt_mp->width;
> +               pix_fmt_mp->plane_fmt[2].bytesperline = 0;
> +       } else if (pix_fmt_mp->num_planes == 3) {
> +               pix_fmt_mp->plane_fmt[1].sizeimage =
> +               pix_fmt_mp->plane_fmt[2].sizeimage =
> +                       (pix_fmt_mp->width * pix_fmt_mp->height) / 4 +
> +                       ((ALIGN(pix_fmt_mp->width, 16) / 2) * 16);
> +               pix_fmt_mp->plane_fmt[1].bytesperline =
> +                       pix_fmt_mp->plane_fmt[2].bytesperline =
> +                       pix_fmt_mp->width / 2;
>         }
>
>         pix_fmt_mp->flags = 0;
> @@ -432,9 +427,7 @@ static int vidioc_venc_s_fmt_cap(struct file *file, void *priv,
>         }
>
>         q_data->fmt = fmt;
> -       ret = vidioc_try_fmt(ctx, f, q_data->fmt);
> -       if (ret)
> -               return ret;
> +       vidioc_try_fmt_cap(f);
>
>         q_data->coded_width = f->fmt.pix_mp.width;
>         q_data->coded_height = f->fmt.pix_mp.height;
> @@ -494,7 +487,7 @@ static int vidioc_venc_s_fmt_out(struct file *file, void *priv,
>                 f->fmt.pix.pixelformat = fmt->fourcc;
>         }
>
> -       ret = vidioc_try_fmt(ctx, f, fmt);
> +       ret = vidioc_try_fmt_out(ctx, f, fmt);
>         if (ret)
>                 return ret;
>
> @@ -572,7 +565,9 @@ static int vidioc_try_fmt_vid_cap_mplane(struct file *file, void *priv,
>         f->fmt.pix_mp.quantization = ctx->quantization;
>         f->fmt.pix_mp.xfer_func = ctx->xfer_func;
>
> -       return vidioc_try_fmt(ctx, f, fmt);
> +       vidioc_try_fmt_cap(f);
> +
> +       return 0;
>  }
>
>  static int vidioc_try_fmt_vid_out_mplane(struct file *file, void *priv,
> @@ -594,7 +589,7 @@ static int vidioc_try_fmt_vid_out_mplane(struct file *file, void *priv,
>                 f->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT;
>         }
>
> -       return vidioc_try_fmt(ctx, f, fmt);
> +       return vidioc_try_fmt_out(ctx, f, fmt);
>  }
>
>  static int vidioc_venc_g_selection(struct file *file, void *priv,
> --
> 2.17.1
>

[PATCH v2 6/7] media: mtk-vcodec: don't check return val of mtk_venc_get_q_data

[Dafna Hirschfeld]

The function mtk_venc_get_q_data always returns a reference
so there is no need to check if the return value is null.
In addition move the q_data initialization to the declaration

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 .../platform/mtk-vcodec/mtk_vcodec_enc.c      | 39 ++++---------------
 1 file changed, 7 insertions(+), 32 deletions(-)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index be28f2401839..5caaeb4773ca 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -399,7 +399,7 @@ static int vidioc_venc_s_fmt_cap(struct file *file, void *priv,
 	struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
 	const struct mtk_vcodec_enc_pdata *pdata = ctx->dev->venc_pdata;
 	struct vb2_queue *vq;
-	struct mtk_q_data *q_data;
+	struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, f->type);
 	int i, ret;
 	const struct mtk_video_fmt *fmt;
 
@@ -414,12 +414,6 @@ static int vidioc_venc_s_fmt_cap(struct file *file, void *priv,
 		return -EBUSY;
 	}
 
-	q_data = mtk_venc_get_q_data(ctx, f->type);
-	if (!q_data) {
-		mtk_v4l2_err("fail to get q data");
-		return -EINVAL;
-	}
-
 	fmt = mtk_venc_find_format(f->fmt.pix.pixelformat, pdata);
 	if (!fmt) {
 		fmt = &ctx->dev->venc_pdata->capture_formats[0];
@@ -460,7 +454,7 @@ static int vidioc_venc_s_fmt_out(struct file *file, void *priv,
 	struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
 	const struct mtk_vcodec_enc_pdata *pdata = ctx->dev->venc_pdata;
 	struct vb2_queue *vq;
-	struct mtk_q_data *q_data;
+	struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, f->type);
 	int ret, i;
 	const struct mtk_video_fmt *fmt;
 
@@ -475,12 +469,6 @@ static int vidioc_venc_s_fmt_out(struct file *file, void *priv,
 		return -EBUSY;
 	}
 
-	q_data = mtk_venc_get_q_data(ctx, f->type);
-	if (!q_data) {
-		mtk_v4l2_err("fail to get q data");
-		return -EINVAL;
-	}
-
 	fmt = mtk_venc_find_format(f->fmt.pix.pixelformat, pdata);
 	if (!fmt) {
 		fmt = &ctx->dev->venc_pdata->output_formats[0];
@@ -520,14 +508,13 @@ static int vidioc_venc_g_fmt(struct file *file, void *priv,
 	struct v4l2_pix_format_mplane *pix = &f->fmt.pix_mp;
 	struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
 	struct vb2_queue *vq;
-	struct mtk_q_data *q_data;
+	struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, f->type);
 	int i;
 
 	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, f->type);
 	if (!vq)
 		return -EINVAL;
 
-	q_data = mtk_venc_get_q_data(ctx, f->type);
 
 	pix->width = q_data->coded_width;
 	pix->height = q_data->coded_height;
@@ -596,15 +583,11 @@ static int vidioc_venc_g_selection(struct file *file, void *priv,
 				     struct v4l2_selection *s)
 {
 	struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
-	struct mtk_q_data *q_data;
+	struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, s->type);
 
 	if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
-	q_data = mtk_venc_get_q_data(ctx, s->type);
-	if (!q_data)
-		return -EINVAL;
-
 	switch (s->target) {
 	case V4L2_SEL_TGT_CROP_DEFAULT:
 	case V4L2_SEL_TGT_CROP_BOUNDS:
@@ -630,15 +613,11 @@ static int vidioc_venc_s_selection(struct file *file, void *priv,
 				     struct v4l2_selection *s)
 {
 	struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
-	struct mtk_q_data *q_data;
+	struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, s->type);
 
 	if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
-	q_data = mtk_venc_get_q_data(ctx, s->type);
-	if (!q_data)
-		return -EINVAL;
-
 	switch (s->target) {
 	case V4L2_SEL_TGT_CROP:
 		/* Only support crop from (0,0) */
@@ -805,11 +784,9 @@ static int vb2ops_venc_queue_setup(struct vb2_queue *vq,
 				   struct device *alloc_devs[])
 {
 	struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vq);
-	struct mtk_q_data *q_data;
+	struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, vq->type);
 	unsigned int i;
 
-	q_data = mtk_venc_get_q_data(ctx, vq->type);
-
 	if (q_data == NULL)
 		return -EINVAL;
 
@@ -829,11 +806,9 @@ static int vb2ops_venc_queue_setup(struct vb2_queue *vq,
 static int vb2ops_venc_buf_prepare(struct vb2_buffer *vb)
 {
 	struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
-	struct mtk_q_data *q_data;
+	struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, vb->vb2_queue->type);
 	int i;
 
-	q_data = mtk_venc_get_q_data(ctx, vb->vb2_queue->type);
-
 	for (i = 0; i < q_data->fmt->num_planes; i++) {
 		if (vb2_plane_size(vb, i) < q_data->sizeimage[i]) {
 			mtk_v4l2_err("data will not fit into plane %d (%lu < %d)",
-- 
2.17.1

Re: [PATCH v2 6/7] media: mtk-vcodec: don't check return val of mtk_venc_get_q_data

[Alexandre Courbot]

On Wed, Nov 17, 2021 at 10:07 PM Dafna Hirschfeld
<dafna.hirschfeld@collabora.com> wrote:
>
> The function mtk_venc_get_q_data always returns a reference
> so there is no need to check if the return value is null.
> In addition move the q_data initialization to the declaration
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

Reviewed-by: Alexandre Courbot <acourbot@chromium.org>


> ---
>  .../platform/mtk-vcodec/mtk_vcodec_enc.c      | 39 ++++---------------
>  1 file changed, 7 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> index be28f2401839..5caaeb4773ca 100644
> --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
> @@ -399,7 +399,7 @@ static int vidioc_venc_s_fmt_cap(struct file *file, void *priv,
>         struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
>         const struct mtk_vcodec_enc_pdata *pdata = ctx->dev->venc_pdata;
>         struct vb2_queue *vq;
> -       struct mtk_q_data *q_data;
> +       struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, f->type);
>         int i, ret;
>         const struct mtk_video_fmt *fmt;
>
> @@ -414,12 +414,6 @@ static int vidioc_venc_s_fmt_cap(struct file *file, void *priv,
>                 return -EBUSY;
>         }
>
> -       q_data = mtk_venc_get_q_data(ctx, f->type);
> -       if (!q_data) {
> -               mtk_v4l2_err("fail to get q data");
> -               return -EINVAL;
> -       }
> -
>         fmt = mtk_venc_find_format(f->fmt.pix.pixelformat, pdata);
>         if (!fmt) {
>                 fmt = &ctx->dev->venc_pdata->capture_formats[0];
> @@ -460,7 +454,7 @@ static int vidioc_venc_s_fmt_out(struct file *file, void *priv,
>         struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
>         const struct mtk_vcodec_enc_pdata *pdata = ctx->dev->venc_pdata;
>         struct vb2_queue *vq;
> -       struct mtk_q_data *q_data;
> +       struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, f->type);
>         int ret, i;
>         const struct mtk_video_fmt *fmt;
>
> @@ -475,12 +469,6 @@ static int vidioc_venc_s_fmt_out(struct file *file, void *priv,
>                 return -EBUSY;
>         }
>
> -       q_data = mtk_venc_get_q_data(ctx, f->type);
> -       if (!q_data) {
> -               mtk_v4l2_err("fail to get q data");
> -               return -EINVAL;
> -       }
> -
>         fmt = mtk_venc_find_format(f->fmt.pix.pixelformat, pdata);
>         if (!fmt) {
>                 fmt = &ctx->dev->venc_pdata->output_formats[0];
> @@ -520,14 +508,13 @@ static int vidioc_venc_g_fmt(struct file *file, void *priv,
>         struct v4l2_pix_format_mplane *pix = &f->fmt.pix_mp;
>         struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
>         struct vb2_queue *vq;
> -       struct mtk_q_data *q_data;
> +       struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, f->type);
>         int i;
>
>         vq = v4l2_m2m_get_vq(ctx->m2m_ctx, f->type);
>         if (!vq)
>                 return -EINVAL;
>
> -       q_data = mtk_venc_get_q_data(ctx, f->type);
>
>         pix->width = q_data->coded_width;
>         pix->height = q_data->coded_height;
> @@ -596,15 +583,11 @@ static int vidioc_venc_g_selection(struct file *file, void *priv,
>                                      struct v4l2_selection *s)
>  {
>         struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
> -       struct mtk_q_data *q_data;
> +       struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, s->type);
>
>         if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
>                 return -EINVAL;
>
> -       q_data = mtk_venc_get_q_data(ctx, s->type);
> -       if (!q_data)
> -               return -EINVAL;
> -
>         switch (s->target) {
>         case V4L2_SEL_TGT_CROP_DEFAULT:
>         case V4L2_SEL_TGT_CROP_BOUNDS:
> @@ -630,15 +613,11 @@ static int vidioc_venc_s_selection(struct file *file, void *priv,
>                                      struct v4l2_selection *s)
>  {
>         struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
> -       struct mtk_q_data *q_data;
> +       struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, s->type);
>
>         if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
>                 return -EINVAL;
>
> -       q_data = mtk_venc_get_q_data(ctx, s->type);
> -       if (!q_data)
> -               return -EINVAL;
> -
>         switch (s->target) {
>         case V4L2_SEL_TGT_CROP:
>                 /* Only support crop from (0,0) */
> @@ -805,11 +784,9 @@ static int vb2ops_venc_queue_setup(struct vb2_queue *vq,
>                                    struct device *alloc_devs[])
>  {
>         struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vq);
> -       struct mtk_q_data *q_data;
> +       struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, vq->type);
>         unsigned int i;
>
> -       q_data = mtk_venc_get_q_data(ctx, vq->type);
> -
>         if (q_data == NULL)
>                 return -EINVAL;
>
> @@ -829,11 +806,9 @@ static int vb2ops_venc_queue_setup(struct vb2_queue *vq,
>  static int vb2ops_venc_buf_prepare(struct vb2_buffer *vb)
>  {
>         struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> -       struct mtk_q_data *q_data;
> +       struct mtk_q_data *q_data = mtk_venc_get_q_data(ctx, vb->vb2_queue->type);
>         int i;
>
> -       q_data = mtk_venc_get_q_data(ctx, vb->vb2_queue->type);
> -
>         for (i = 0; i < q_data->fmt->num_planes; i++) {
>                 if (vb2_plane_size(vb, i) < q_data->sizeimage[i]) {
>                         mtk_v4l2_err("data will not fit into plane %d (%lu < %d)",
> --
> 2.17.1
>

[PATCH v2 7/7] meida: mtk-vcodec: remove unused func parameter

[Dafna Hirschfeld]

The prarameter bs_size to function vpu_enc_encode
is not used. Remove it.

Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
 drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c | 9 +++------
 drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c  | 3 +--
 drivers/media/platform/mtk-vcodec/venc_vpu_if.c       | 1 -
 drivers/media/platform/mtk-vcodec/venc_vpu_if.h       | 1 -
 4 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
index b6a4f2074fa5..bf03888a824f 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
@@ -367,8 +367,7 @@ static int h264_encode_sps(struct venc_h264_inst *inst,
 
 	mtk_vcodec_debug_enter(inst);
 
-	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_SPS, NULL,
-			     bs_buf, bs_size, NULL);
+	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_SPS, NULL, bs_buf, NULL);
 	if (ret)
 		return ret;
 
@@ -394,8 +393,7 @@ static int h264_encode_pps(struct venc_h264_inst *inst,
 
 	mtk_vcodec_debug_enter(inst);
 
-	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_PPS, NULL,
-			     bs_buf, bs_size, NULL);
+	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_PPS, NULL, bs_buf, NULL);
 	if (ret)
 		return ret;
 
@@ -451,8 +449,7 @@ static int h264_encode_frame(struct venc_h264_inst *inst,
 	mtk_vcodec_debug(inst, "frm_count = %d,skip_frm_count =%d,frm_type=%d.\n",
 			 frame_info.frm_count, frame_info.skip_frm_count,
 			 frame_info.frm_type);
-	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_FRAME, frm_buf,
-			     bs_buf, bs_size, &frame_info);
+	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_FRAME, frm_buf, bs_buf, &frame_info);
 	if (ret)
 		return ret;
 
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
index 8267a9c4fd25..6b66957d5192 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
@@ -302,8 +302,7 @@ static int vp8_enc_encode_frame(struct venc_vp8_inst *inst,
 
 	mtk_vcodec_debug(inst, "->frm_cnt=%d", inst->frm_cnt);
 
-	ret = vpu_enc_encode(&inst->vpu_inst, 0, frm_buf, bs_buf, bs_size,
-			     NULL);
+	ret = vpu_enc_encode(&inst->vpu_inst, 0, frm_buf, bs_buf, NULL);
 	if (ret)
 		return ret;
 
diff --git a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
index be6d8790a41e..e7899d8a3e4e 100644
--- a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
@@ -225,7 +225,6 @@ int vpu_enc_set_param(struct venc_vpu_inst *vpu,
 int vpu_enc_encode(struct venc_vpu_inst *vpu, unsigned int bs_mode,
 		   struct venc_frm_buf *frm_buf,
 		   struct mtk_vcodec_mem *bs_buf,
-		   unsigned int *bs_size,
 		   struct venc_frame_info *frame_info)
 {
 	const bool is_ext = MTK_ENC_CTX_IS_EXT(vpu->ctx);
diff --git a/drivers/media/platform/mtk-vcodec/venc_vpu_if.h b/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
index f9be9cab7ff7..f83bc1b3f2bf 100644
--- a/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
+++ b/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
@@ -45,7 +45,6 @@ int vpu_enc_set_param(struct venc_vpu_inst *vpu,
 int vpu_enc_encode(struct venc_vpu_inst *vpu, unsigned int bs_mode,
 		   struct venc_frm_buf *frm_buf,
 		   struct mtk_vcodec_mem *bs_buf,
-		   unsigned int *bs_size,
 		   struct venc_frame_info *frame_info);
 int vpu_enc_deinit(struct venc_vpu_inst *vpu);
 
-- 
2.17.1

Re: [PATCH v2 7/7] meida: mtk-vcodec: remove unused func parameter

[Nicolas Dufresne]

Just noticed the headline typo: meida -> media

cheers,
Nicolas

Le mercredi 17 novembre 2021 à 15:06 +0200, Dafna Hirschfeld a écrit :
> The prarameter bs_size to function vpu_enc_encode
> is not used. Remove it.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c | 9 +++------
>  drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c  | 3 +--
>  drivers/media/platform/mtk-vcodec/venc_vpu_if.c       | 1 -
>  drivers/media/platform/mtk-vcodec/venc_vpu_if.h       | 1 -
>  4 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
> index b6a4f2074fa5..bf03888a824f 100644
> --- a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
> @@ -367,8 +367,7 @@ static int h264_encode_sps(struct venc_h264_inst *inst,
>  
>  	mtk_vcodec_debug_enter(inst);
>  
> -	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_SPS, NULL,
> -			     bs_buf, bs_size, NULL);
> +	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_SPS, NULL, bs_buf, NULL);
>  	if (ret)
>  		return ret;
>  
> @@ -394,8 +393,7 @@ static int h264_encode_pps(struct venc_h264_inst *inst,
>  
>  	mtk_vcodec_debug_enter(inst);
>  
> -	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_PPS, NULL,
> -			     bs_buf, bs_size, NULL);
> +	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_PPS, NULL, bs_buf, NULL);
>  	if (ret)
>  		return ret;
>  
> @@ -451,8 +449,7 @@ static int h264_encode_frame(struct venc_h264_inst *inst,
>  	mtk_vcodec_debug(inst, "frm_count = %d,skip_frm_count =%d,frm_type=%d.\n",
>  			 frame_info.frm_count, frame_info.skip_frm_count,
>  			 frame_info.frm_type);
> -	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_FRAME, frm_buf,
> -			     bs_buf, bs_size, &frame_info);
> +	ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_FRAME, frm_buf, bs_buf, &frame_info);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> index 8267a9c4fd25..6b66957d5192 100644
> --- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> @@ -302,8 +302,7 @@ static int vp8_enc_encode_frame(struct venc_vp8_inst *inst,
>  
>  	mtk_vcodec_debug(inst, "->frm_cnt=%d", inst->frm_cnt);
>  
> -	ret = vpu_enc_encode(&inst->vpu_inst, 0, frm_buf, bs_buf, bs_size,
> -			     NULL);
> +	ret = vpu_enc_encode(&inst->vpu_inst, 0, frm_buf, bs_buf, NULL);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> index be6d8790a41e..e7899d8a3e4e 100644
> --- a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> @@ -225,7 +225,6 @@ int vpu_enc_set_param(struct venc_vpu_inst *vpu,
>  int vpu_enc_encode(struct venc_vpu_inst *vpu, unsigned int bs_mode,
>  		   struct venc_frm_buf *frm_buf,
>  		   struct mtk_vcodec_mem *bs_buf,
> -		   unsigned int *bs_size,
>  		   struct venc_frame_info *frame_info)
>  {
>  	const bool is_ext = MTK_ENC_CTX_IS_EXT(vpu->ctx);
> diff --git a/drivers/media/platform/mtk-vcodec/venc_vpu_if.h b/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
> index f9be9cab7ff7..f83bc1b3f2bf 100644
> --- a/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
> +++ b/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
> @@ -45,7 +45,6 @@ int vpu_enc_set_param(struct venc_vpu_inst *vpu,
>  int vpu_enc_encode(struct venc_vpu_inst *vpu, unsigned int bs_mode,
>  		   struct venc_frm_buf *frm_buf,
>  		   struct mtk_vcodec_mem *bs_buf,
> -		   unsigned int *bs_size,
>  		   struct venc_frame_info *frame_info);
>  int vpu_enc_deinit(struct venc_vpu_inst *vpu);
>  

Re: [PATCH v2 7/7] meida: mtk-vcodec: remove unused func parameter

[Alexandre Courbot]

On Wed, Nov 17, 2021 at 10:07 PM Dafna Hirschfeld
<dafna.hirschfeld@collabora.com> wrote:
>
> The prarameter bs_size to function vpu_enc_encode
> is not used. Remove it.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

Indeed, it's an output parameter of the calling functions and has no
business being passed to vpu_enc_encode.

With the typo in the headline fixed,

Reviewed-by: Alexandre Courbot <acourbot@chromium.org>


> ---
>  drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c | 9 +++------
>  drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c  | 3 +--
>  drivers/media/platform/mtk-vcodec/venc_vpu_if.c       | 1 -
>  drivers/media/platform/mtk-vcodec/venc_vpu_if.h       | 1 -
>  4 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
> index b6a4f2074fa5..bf03888a824f 100644
> --- a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
> @@ -367,8 +367,7 @@ static int h264_encode_sps(struct venc_h264_inst *inst,
>
>         mtk_vcodec_debug_enter(inst);
>
> -       ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_SPS, NULL,
> -                            bs_buf, bs_size, NULL);
> +       ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_SPS, NULL, bs_buf, NULL);
>         if (ret)
>                 return ret;
>
> @@ -394,8 +393,7 @@ static int h264_encode_pps(struct venc_h264_inst *inst,
>
>         mtk_vcodec_debug_enter(inst);
>
> -       ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_PPS, NULL,
> -                            bs_buf, bs_size, NULL);
> +       ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_PPS, NULL, bs_buf, NULL);
>         if (ret)
>                 return ret;
>
> @@ -451,8 +449,7 @@ static int h264_encode_frame(struct venc_h264_inst *inst,
>         mtk_vcodec_debug(inst, "frm_count = %d,skip_frm_count =%d,frm_type=%d.\n",
>                          frame_info.frm_count, frame_info.skip_frm_count,
>                          frame_info.frm_type);
> -       ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_FRAME, frm_buf,
> -                            bs_buf, bs_size, &frame_info);
> +       ret = vpu_enc_encode(&inst->vpu_inst, H264_BS_MODE_FRAME, frm_buf, bs_buf, &frame_info);
>         if (ret)
>                 return ret;
>
> diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> index 8267a9c4fd25..6b66957d5192 100644
> --- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> @@ -302,8 +302,7 @@ static int vp8_enc_encode_frame(struct venc_vp8_inst *inst,
>
>         mtk_vcodec_debug(inst, "->frm_cnt=%d", inst->frm_cnt);
>
> -       ret = vpu_enc_encode(&inst->vpu_inst, 0, frm_buf, bs_buf, bs_size,
> -                            NULL);
> +       ret = vpu_enc_encode(&inst->vpu_inst, 0, frm_buf, bs_buf, NULL);
>         if (ret)
>                 return ret;
>
> diff --git a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> index be6d8790a41e..e7899d8a3e4e 100644
> --- a/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc_vpu_if.c
> @@ -225,7 +225,6 @@ int vpu_enc_set_param(struct venc_vpu_inst *vpu,
>  int vpu_enc_encode(struct venc_vpu_inst *vpu, unsigned int bs_mode,
>                    struct venc_frm_buf *frm_buf,
>                    struct mtk_vcodec_mem *bs_buf,
> -                  unsigned int *bs_size,
>                    struct venc_frame_info *frame_info)
>  {
>         const bool is_ext = MTK_ENC_CTX_IS_EXT(vpu->ctx);
> diff --git a/drivers/media/platform/mtk-vcodec/venc_vpu_if.h b/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
> index f9be9cab7ff7..f83bc1b3f2bf 100644
> --- a/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
> +++ b/drivers/media/platform/mtk-vcodec/venc_vpu_if.h
> @@ -45,7 +45,6 @@ int vpu_enc_set_param(struct venc_vpu_inst *vpu,
>  int vpu_enc_encode(struct venc_vpu_inst *vpu, unsigned int bs_mode,
>                    struct venc_frm_buf *frm_buf,
>                    struct mtk_vcodec_mem *bs_buf,
> -                  unsigned int *bs_size,
>                    struct venc_frame_info *frame_info);
>  int vpu_enc_deinit(struct venc_vpu_inst *vpu);
>
> --
> 2.17.1
>