[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 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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[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 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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[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

[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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[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

[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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[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 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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[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

[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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[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

[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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[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

[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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

[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);
>  


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 10434 bytes --]

Hi Dafna,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on linux/master linus/master v5.16-rc1 next-20211118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dafna-Hirschfeld/media-mtk-vcodec-few-fixes/20211117-210840
base:   git://linuxtv.org/media_tree.git master
config: arm-buildonly-randconfig-r003-20211118 (attached as .config)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/87678e46cdd12f69edad572f8561e8ee929d1d16
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dafna-Hirschfeld/media-mtk-vcodec-few-fixes/20211117-210840
        git checkout 87678e46cdd12f69edad572f8561e8ee929d1d16
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:641:5: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                            sizeof(struct mtk_h264_sps_param),
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h:46:46: note: expanded from macro 'mtk_vcodec_debug'
                   ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
                                                              ^~~~
   include/linux/printk.h:570:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:642:5: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                            sizeof(struct mtk_h264_pps_param),
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h:46:46: note: expanded from macro 'mtk_vcodec_debug'
                   ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
                                                              ^~~~
   include/linux/printk.h:570:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:643:5: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                            sizeof(struct mtk_h264_dec_slice_param),
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h:46:46: note: expanded from macro 'mtk_vcodec_debug'
                   ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
                                                              ^~~~
   include/linux/printk.h:570:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:644:5: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                            sizeof(struct mtk_h264_dpb_info));
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h:46:46: note: expanded from macro 'mtk_vcodec_debug'
                   ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
                                                              ^~~~
   include/linux/printk.h:570:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   4 warnings generated.


vim +641 drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c

06fa5f757dc5a5 Yunfei Dong      2021-08-06  611  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  612  static int vdec_h264_slice_init(struct mtk_vcodec_ctx *ctx)
06fa5f757dc5a5 Yunfei Dong      2021-08-06  613  {
06fa5f757dc5a5 Yunfei Dong      2021-08-06  614  	struct vdec_h264_slice_inst *inst;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  615  	int err;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  616  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  617  	inst = kzalloc(sizeof(*inst), GFP_KERNEL);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  618  	if (!inst)
06fa5f757dc5a5 Yunfei Dong      2021-08-06  619  		return -ENOMEM;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  620  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  621  	inst->ctx = ctx;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  622  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  623  	inst->vpu.id = SCP_IPI_VDEC_H264;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  624  	inst->vpu.ctx = ctx;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  625  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  626  	err = vpu_dec_init(&inst->vpu);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  627  	if (err) {
06fa5f757dc5a5 Yunfei Dong      2021-08-06  628  		mtk_vcodec_err(inst, "vdec_h264 init err=%d", err);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  629  		goto error_free_inst;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  630  	}
06fa5f757dc5a5 Yunfei Dong      2021-08-06  631  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  632  	memcpy(&inst->vsi_ctx, inst->vpu.vsi, sizeof(inst->vsi_ctx));
06fa5f757dc5a5 Yunfei Dong      2021-08-06  633  	inst->vsi_ctx.dec.resolution_changed = true;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  634  	inst->vsi_ctx.dec.realloc_mv_buf = true;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  635  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  636  	err = allocate_predication_buf(inst);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  637  	if (err)
06fa5f757dc5a5 Yunfei Dong      2021-08-06  638  		goto error_deinit;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  639  
87678e46cdd12f Dafna Hirschfeld 2021-11-17  640  	mtk_vcodec_debug(inst, "struct size = %lu,%lu,%lu,%lu\n",
06fa5f757dc5a5 Yunfei Dong      2021-08-06 @641  			 sizeof(struct mtk_h264_sps_param),
06fa5f757dc5a5 Yunfei Dong      2021-08-06  642  			 sizeof(struct mtk_h264_pps_param),
06fa5f757dc5a5 Yunfei Dong      2021-08-06  643  			 sizeof(struct mtk_h264_dec_slice_param),
06fa5f757dc5a5 Yunfei Dong      2021-08-06  644  			 sizeof(struct mtk_h264_dpb_info));
06fa5f757dc5a5 Yunfei Dong      2021-08-06  645  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  646  	mtk_vcodec_debug(inst, "H264 Instance >> %p", inst);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  647  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  648  	ctx->drv_handle = inst;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  649  	return 0;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  650  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  651  error_deinit:
06fa5f757dc5a5 Yunfei Dong      2021-08-06  652  	vpu_dec_deinit(&inst->vpu);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  653  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  654  error_free_inst:
06fa5f757dc5a5 Yunfei Dong      2021-08-06  655  	kfree(inst);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  656  	return err;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  657  }
06fa5f757dc5a5 Yunfei Dong      2021-08-06  658  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38996 bytes --]

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 10595 bytes --]

Hi Dafna,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on linux/master linus/master v5.16-rc1 next-20211118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dafna-Hirschfeld/media-mtk-vcodec-few-fixes/20211117-210840
base:   git://linuxtv.org/media_tree.git master
config: arm-buildonly-randconfig-r003-20211118 (attached as .config)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/87678e46cdd12f69edad572f8561e8ee929d1d16
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dafna-Hirschfeld/media-mtk-vcodec-few-fixes/20211117-210840
        git checkout 87678e46cdd12f69edad572f8561e8ee929d1d16
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:641:5: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                            sizeof(struct mtk_h264_sps_param),
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h:46:46: note: expanded from macro 'mtk_vcodec_debug'
                   ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
                                                              ^~~~
   include/linux/printk.h:570:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:642:5: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                            sizeof(struct mtk_h264_pps_param),
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h:46:46: note: expanded from macro 'mtk_vcodec_debug'
                   ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
                                                              ^~~~
   include/linux/printk.h:570:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:643:5: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                            sizeof(struct mtk_h264_dec_slice_param),
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h:46:46: note: expanded from macro 'mtk_vcodec_debug'
                   ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
                                                              ^~~~
   include/linux/printk.h:570:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:644:5: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
                            sizeof(struct mtk_h264_dpb_info));
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/mtk-vcodec/mtk_vcodec_util.h:46:46: note: expanded from macro 'mtk_vcodec_debug'
                   ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
                                                              ^~~~
   include/linux/printk.h:570:26: note: expanded from macro 'pr_debug'
           dynamic_pr_debug(fmt, ##__VA_ARGS__)
                            ~~~    ^~~~~~~~~~~
   include/linux/dynamic_debug.h:163:22: note: expanded from macro 'dynamic_pr_debug'
                              pr_fmt(fmt), ##__VA_ARGS__)
                                     ~~~     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call'
           __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
                                                                 ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call'
                   func(&id, ##__VA_ARGS__);               \
                               ^~~~~~~~~~~
   4 warnings generated.


vim +641 drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c

06fa5f757dc5a5 Yunfei Dong      2021-08-06  611  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  612  static int vdec_h264_slice_init(struct mtk_vcodec_ctx *ctx)
06fa5f757dc5a5 Yunfei Dong      2021-08-06  613  {
06fa5f757dc5a5 Yunfei Dong      2021-08-06  614  	struct vdec_h264_slice_inst *inst;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  615  	int err;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  616  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  617  	inst = kzalloc(sizeof(*inst), GFP_KERNEL);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  618  	if (!inst)
06fa5f757dc5a5 Yunfei Dong      2021-08-06  619  		return -ENOMEM;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  620  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  621  	inst->ctx = ctx;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  622  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  623  	inst->vpu.id = SCP_IPI_VDEC_H264;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  624  	inst->vpu.ctx = ctx;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  625  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  626  	err = vpu_dec_init(&inst->vpu);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  627  	if (err) {
06fa5f757dc5a5 Yunfei Dong      2021-08-06  628  		mtk_vcodec_err(inst, "vdec_h264 init err=%d", err);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  629  		goto error_free_inst;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  630  	}
06fa5f757dc5a5 Yunfei Dong      2021-08-06  631  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  632  	memcpy(&inst->vsi_ctx, inst->vpu.vsi, sizeof(inst->vsi_ctx));
06fa5f757dc5a5 Yunfei Dong      2021-08-06  633  	inst->vsi_ctx.dec.resolution_changed = true;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  634  	inst->vsi_ctx.dec.realloc_mv_buf = true;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  635  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  636  	err = allocate_predication_buf(inst);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  637  	if (err)
06fa5f757dc5a5 Yunfei Dong      2021-08-06  638  		goto error_deinit;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  639  
87678e46cdd12f Dafna Hirschfeld 2021-11-17  640  	mtk_vcodec_debug(inst, "struct size = %lu,%lu,%lu,%lu\n",
06fa5f757dc5a5 Yunfei Dong      2021-08-06 @641  			 sizeof(struct mtk_h264_sps_param),
06fa5f757dc5a5 Yunfei Dong      2021-08-06  642  			 sizeof(struct mtk_h264_pps_param),
06fa5f757dc5a5 Yunfei Dong      2021-08-06  643  			 sizeof(struct mtk_h264_dec_slice_param),
06fa5f757dc5a5 Yunfei Dong      2021-08-06  644  			 sizeof(struct mtk_h264_dpb_info));
06fa5f757dc5a5 Yunfei Dong      2021-08-06  645  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  646  	mtk_vcodec_debug(inst, "H264 Instance >> %p", inst);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  647  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  648  	ctx->drv_handle = inst;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  649  	return 0;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  650  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  651  error_deinit:
06fa5f757dc5a5 Yunfei Dong      2021-08-06  652  	vpu_dec_deinit(&inst->vpu);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  653  
06fa5f757dc5a5 Yunfei Dong      2021-08-06  654  error_free_inst:
06fa5f757dc5a5 Yunfei Dong      2021-08-06  655  	kfree(inst);
06fa5f757dc5a5 Yunfei Dong      2021-08-06  656  	return err;
06fa5f757dc5a5 Yunfei Dong      2021-08-06  657  }
06fa5f757dc5a5 Yunfei Dong      2021-08-06  658  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38996 bytes --]

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)
> 


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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)
> 

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 8995 bytes --]

Hi Dafna,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on linux/master linus/master v5.16-rc2]
[cannot apply to next-20211126]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dafna-Hirschfeld/media-mtk-vcodec-few-fixes/20211117-210840
base:   git://linuxtv.org/media_tree.git master
config: arc-randconfig-r023-20211117 (https://download.01.org/0day-ci/archive/20211126/202111261657.wV6tp8uQ-lkp(a)intel.com/config)
compiler: arc-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/87678e46cdd12f69edad572f8561e8ee929d1d16
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dafna-Hirschfeld/media-mtk-vcodec-few-fixes/20211117-210840
        git checkout 87678e46cdd12f69edad572f8561e8ee929d1d16
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:19,
                    from include/linux/list.h:9,
                    from include/linux/module.h:12,
                    from drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:3:
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c: In function 'vdec_h264_slice_init':
>> include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:132:17: note: in expansion of macro 'printk'
     132 |                 printk(fmt, ##__VA_ARGS__);             \
         |                 ^~~~~~
   include/linux/printk.h:576:9: note: in expansion of macro 'no_printk'
     576 |         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~
   include/linux/kern_levels.h:15:25: note: in expansion of macro 'KERN_SOH'
      15 | #define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */
         |                         ^~~~~~~~
   include/linux/printk.h:576:19: note: in expansion of macro 'KERN_DEBUG'
     576 |         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
         |                   ^~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/../mtk_vcodec_util.h:45:9: note: in expansion of macro 'pr_debug'
      45 |         pr_debug("[MTK_VCODEC][%d]: " fmt "\n",                 \
         |         ^~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:640:9: note: in expansion of macro 'mtk_vcodec_debug'
     640 |         mtk_vcodec_debug(inst, "struct size = %lu,%lu,%lu,%lu\n",
         |         ^~~~~~~~~~~~~~~~
   include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'unsigned int' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:132:17: note: in expansion of macro 'printk'
     132 |                 printk(fmt, ##__VA_ARGS__);             \
         |                 ^~~~~~
   include/linux/printk.h:576:9: note: in expansion of macro 'no_printk'
     576 |         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~
   include/linux/kern_levels.h:15:25: note: in expansion of macro 'KERN_SOH'
      15 | #define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */
         |                         ^~~~~~~~
   include/linux/printk.h:576:19: note: in expansion of macro 'KERN_DEBUG'
     576 |         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
         |                   ^~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/../mtk_vcodec_util.h:45:9: note: in expansion of macro 'pr_debug'
      45 |         pr_debug("[MTK_VCODEC][%d]: " fmt "\n",                 \
         |         ^~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:640:9: note: in expansion of macro 'mtk_vcodec_debug'
     640 |         mtk_vcodec_debug(inst, "struct size = %lu,%lu,%lu,%lu\n",
         |         ^~~~~~~~~~~~~~~~
   include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 7 has type 'unsigned int' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:132:17: note: in expansion of macro 'printk'
     132 |                 printk(fmt, ##__VA_ARGS__);             \
         |                 ^~~~~~
   include/linux/printk.h:576:9: note: in expansion of macro 'no_printk'
     576 |         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~
   include/linux/kern_levels.h:15:25: note: in expansion of macro 'KERN_SOH'
      15 | #define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */
         |                         ^~~~~~~~
   include/linux/printk.h:576:19: note: in expansion of macro 'KERN_DEBUG'
     576 |         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
         |                   ^~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/../mtk_vcodec_util.h:45:9: note: in expansion of macro 'pr_debug'
      45 |         pr_debug("[MTK_VCODEC][%d]: " fmt "\n",                 \
         |         ^~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:640:9: note: in expansion of macro 'mtk_vcodec_debug'
     640 |         mtk_vcodec_debug(inst, "struct size = %lu,%lu,%lu,%lu\n",
         |         ^~~~~~~~~~~~~~~~
   include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 8 has type 'unsigned int' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:132:17: note: in expansion of macro 'printk'
     132 |                 printk(fmt, ##__VA_ARGS__);             \
         |                 ^~~~~~
   include/linux/printk.h:576:9: note: in expansion of macro 'no_printk'
     576 |         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~~~~
   include/linux/kern_levels.h:15:25: note: in expansion of macro 'KERN_SOH'
      15 | #define KERN_DEBUG      KERN_SOH "7"    /* debug-level messages */
         |                         ^~~~~~~~
   include/linux/printk.h:576:19: note: in expansion of macro 'KERN_DEBUG'
     576 |         no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
         |                   ^~~~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/../mtk_vcodec_util.h:45:9: note: in expansion of macro 'pr_debug'
      45 |         pr_debug("[MTK_VCODEC][%d]: " fmt "\n",                 \
         |         ^~~~~~~~
   drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c:640:9: note: in expansion of macro 'mtk_vcodec_debug'
     640 |         mtk_vcodec_debug(inst, "struct size = %lu,%lu,%lu,%lu\n",
         |         ^~~~~~~~~~~~~~~~


vim +5 include/linux/kern_levels.h

314ba3520e513a7 Joe Perches 2012-07-30  4  
04d2c8c83d0e3ac Joe Perches 2012-07-30 @5  #define KERN_SOH	"\001"		/* ASCII Start Of Header */
04d2c8c83d0e3ac Joe Perches 2012-07-30  6  #define KERN_SOH_ASCII	'\001'
04d2c8c83d0e3ac Joe Perches 2012-07-30  7  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

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
>

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
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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
>

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
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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
>

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
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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
>

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
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek