quoted

decoder firmware may notify host some debug message, it can help
analyze the state of the firmware in case of error

Fixes: 9f599f351e86 ("media: amphion: add vpu core driver")
Signed-off-by: Ming Qian <ming.qian@nxp.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
v2
- force to terminate string message data
 drivers/media/platform/amphion/vpu_defs.h    | 1 +
 drivers/media/platform/amphion/vpu_helpers.c | 1 +
drivers/media/platform/amphion/vpu_malone.c  | 1 +
 drivers/media/platform/amphion/vpu_msgs.c    | 9 +++++++++
 drivers/media/platform/amphion/vpu_rpc.c     | 3 ++-
 5 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/amphion/vpu_defs.h

b/drivers/media/platform/amphion/vpu_defs.h
index 667637eedb5d..7320852668d6 100644

--- a/drivers/media/platform/amphion/vpu_defs.h
+++ b/drivers/media/platform/amphion/vpu_defs.h

@@ -71,6 +71,7 @@ enum {
      VPU_MSG_ID_TIMESTAMP_INFO,
      VPU_MSG_ID_FIRMWARE_XCPT,
      VPU_MSG_ID_PIC_SKIPPED,
+     VPU_MSG_ID_DBG_MSG,
 };

 enum VPU_ENC_MEMORY_RESOURSE {

diff --git a/drivers/media/platform/amphion/vpu_helpers.c

b/drivers/media/platform/amphion/vpu_helpers.c
index af3b336e5dc3..d12310af9ebc 100644

--- a/drivers/media/platform/amphion/vpu_helpers.c
+++ b/drivers/media/platform/amphion/vpu_helpers.c

@@ -489,6 +489,7 @@ const char *vpu_id_name(u32 id)
      case VPU_MSG_ID_UNSUPPORTED: return "unsupported";
      case VPU_MSG_ID_FIRMWARE_XCPT: return "exception";
      case VPU_MSG_ID_PIC_SKIPPED: return "skipped";
+     case VPU_MSG_ID_DBG_MSG: return "debug msg";
      }
      return "<unknown>";
 }

diff --git a/drivers/media/platform/amphion/vpu_malone.c

b/drivers/media/platform/amphion/vpu_malone.c
index f771661980c0..d3425de7bccd 100644

--- a/drivers/media/platform/amphion/vpu_malone.c
+++ b/drivers/media/platform/amphion/vpu_malone.c

@@ -745,6 +745,7 @@ static struct vpu_pair malone_msgs[] = {
      {VPU_MSG_ID_UNSUPPORTED,

VID_API_EVENT_UNSUPPORTED_STREAM},

quoted

      {VPU_MSG_ID_FIRMWARE_XCPT, VID_API_EVENT_FIRMWARE_XCPT},
      {VPU_MSG_ID_PIC_SKIPPED, VID_API_EVENT_PIC_SKIPPED},
+     {VPU_MSG_ID_DBG_MSG, VID_API_EVENT_DBG_MSG_DEC},
 };

 static void vpu_malone_pack_fs_alloc(struct vpu_rpc_event *pkt, diff
--git a/drivers/media/platform/amphion/vpu_msgs.c
b/drivers/media/platform/amphion/vpu_msgs.c
index d0ead051f7d1..999efeaebfd5 100644

--- a/drivers/media/platform/amphion/vpu_msgs.c
+++ b/drivers/media/platform/amphion/vpu_msgs.c

@@ -180,6 +180,14 @@ static void vpu_session_handle_pic_skipped(struct

vpu_inst *inst, struct vpu_rpc

quoted

      vpu_inst_unlock(inst);
 }

+static void vpu_session_handle_dbg_msg(struct vpu_inst *inst, struct
+vpu_rpc_event *pkt) {
+     char *str = (char *)pkt->data;
+
+     if (strlen(str))

This can be replaced by: 'if (*str)', it's faster than strlen.

quoted

+             dev_info(inst->dev, "instance %d firmware dbg msg :
+%s\n", inst->id, str); }
+
 static struct vpu_msg_handler handlers[] = {
      {VPU_MSG_ID_START_DONE, vpu_session_handle_start_done},
      {VPU_MSG_ID_STOP_DONE, vpu_session_handle_stop_done}, @@ -

196,6

quoted

+204,7 @@ static struct vpu_msg_handler handlers[] = {
      {VPU_MSG_ID_UNSUPPORTED, vpu_session_handle_error},
      {VPU_MSG_ID_FIRMWARE_XCPT, vpu_session_handle_firmware_xcpt},
      {VPU_MSG_ID_PIC_SKIPPED, vpu_session_handle_pic_skipped},
+     {VPU_MSG_ID_DBG_MSG, vpu_session_handle_dbg_msg},
 };

 static int vpu_session_handle_msg(struct vpu_inst *inst, struct
vpu_rpc_event *msg) diff --git
a/drivers/media/platform/amphion/vpu_rpc.c
b/drivers/media/platform/amphion/vpu_rpc.c
index f626a9f835e0..8234370afee5 100644

--- a/drivers/media/platform/amphion/vpu_rpc.c
+++ b/drivers/media/platform/amphion/vpu_rpc.c

@@ -144,7 +144,7 @@ static int vpu_rpc_receive_msg_buf(struct

vpu_shared_addr *shared, struct vpu_rp

quoted

      msg->hdr.num = (msgword >> 16) & 0xff;
      msg->hdr.id = msgword & 0x3fff;

-     if (msg->hdr.num > ARRAY_SIZE(msg->data))
+     if (msg->hdr.num >= ARRAY_SIZE(msg->data))

Is this right? You are changing the max length just to be able to set the last
byte to 0, which makes sense for VPU_MSG_ID_DBG_MSG but perhaps not for
others.

I think it would be better to leave this code as-is and instead set the last byte
to 0 in vpu_session_handle_dbg_msg(). That's where it is actually needed.

Or am I missing something here?

Regards,

       Hans

It will be fixed in v3

quoted

              return -EINVAL;

      for (i = 0; i < msg->hdr.num; i++) { @@ -156,6 +156,7 @@ static
int vpu_rpc_receive_msg_buf(struct vpu_shared_addr *shared, struct

vpu_rp

quoted

                      data = shared->msg_mem_vir;
              }
      }
+     msg->data[msg->hdr.num] = 0;

      /*update rptr after data is read*/
      mb();

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel