On Fri, May 21, 2021 at 10:37 PM Tzung-Bi Shih [off-list ref] wrote:

On Wed, May 19, 2021 at 10:31 PM Alexandre Courbot
[off-list ref] wrote:

quoted

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateful.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateful.c
new file mode 100644
index 000000000000..ed4b1308a0e4
--- /dev/null
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateful.c

@@ -0,0 +1,667 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <media/v4l2-event.h>
+#include <media/v4l2-mem2mem.h>
+#include <media/videobuf2-dma-contig.h>
+
+#include "mtk_vcodec_drv.h"
+#include "mtk_vcodec_dec.h"
+#include "mtk_vcodec_intr.h"
+#include "mtk_vcodec_util.h"
+#include "vdec_drv_if.h"
+#include "mtk_vcodec_dec_pm.h"

Would be good practice to sort them.

Did what I could, but the order seems to somehow matter. :/

quoted

+static struct vb2_buffer *get_display_buffer(struct mtk_vcodec_ctx *ctx)
+{
+       struct vdec_fb *disp_frame_buffer = NULL;
+       struct mtk_video_dec_buf *dstbuf;
+       struct vb2_v4l2_buffer *vb;
+
+       mtk_v4l2_debug(3, "[%d]", ctx->id);
+       if (vdec_if_get_param(ctx,
+                       GET_PARAM_DISP_FRAME_BUFFER,
+                       &disp_frame_buffer)) {
+               mtk_v4l2_err("[%d]Cannot get param : GET_PARAM_DISP_FRAME_BUFFER",
+                       ctx->id);
+               return NULL;
+       }
+
+       if (disp_frame_buffer == NULL) {

Although this is a move refactor, to be neat, !disp_frame_buffer.

These were also raised by checkpatch.pl --strict, they are now all fixed.

quoted

+static struct vb2_buffer *get_free_buffer(struct mtk_vcodec_ctx *ctx)
+{
+       struct mtk_video_dec_buf *dstbuf;
+       struct vdec_fb *free_frame_buffer = NULL;
+       struct vb2_v4l2_buffer *vb;
+
+       if (vdec_if_get_param(ctx,
+                               GET_PARAM_FREE_FRAME_BUFFER,
+                               &free_frame_buffer)) {
+               mtk_v4l2_err("[%d] Error!! Cannot get param", ctx->id);
+               return NULL;
+       }
+       if (free_frame_buffer == NULL) {

The same concern as above.

quoted

+static void clean_display_buffer(struct mtk_vcodec_ctx *ctx)
+{
+       struct vb2_buffer *framptr;
+
+       do {
+               framptr = get_display_buffer(ctx);
+       } while (framptr);
+}
+
+static void clean_free_buffer(struct mtk_vcodec_ctx *ctx)
+{
+       struct vb2_buffer *framptr;
+
+       do {
+               framptr = get_free_buffer(ctx);
+       } while (framptr);
+}

while (get_display_buffer(ctx)) ;
and
while (get_free_buffer(ctx)) ;
looks better.

Done.

quoted

+static int mtk_vdec_flush_decoder(struct mtk_vcodec_ctx *ctx)
+{
+       bool res_chg;
+       int ret = 0;

No need to initialize.

Done.

quoted

+static void mtk_vdec_worker(struct work_struct *work)
+{
+       struct mtk_vcodec_ctx *ctx = container_of(work, struct mtk_vcodec_ctx,
+                               decode_work);
+       struct mtk_vcodec_dev *dev = ctx->dev;
+       struct vb2_v4l2_buffer *src_buf, *dst_buf;
+       struct mtk_vcodec_mem buf;
+       struct vdec_fb *pfb;
+       bool res_chg = false;
+       int ret;
+       struct mtk_video_dec_buf *dst_buf_info, *src_buf_info;
+
+       src_buf = v4l2_m2m_next_src_buf(ctx->m2m_ctx);
+       if (src_buf == NULL) {

To neat, !src_buf.

quoted

+       dst_buf = v4l2_m2m_next_dst_buf(ctx->m2m_ctx);
+       if (dst_buf == NULL) {

To neat, !dst_buf.

quoted

+static void vb2ops_vdec_stateful_buf_queue(struct vb2_buffer *vb)
+{
+       struct vb2_v4l2_buffer *src_buf;
+       struct mtk_vcodec_mem src_mem;
+       bool res_chg = false;
+       int ret = 0;
+       unsigned int dpbsize = 1, i = 0;
+       struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+       struct vb2_v4l2_buffer *vb2_v4l2 = NULL;

ret, i, and vb2_v4l2 are obviously no need to initialize.

Done, thanks!

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