diff mbox series

media: amphion: handle firmware debug message

Message ID 20230822055207.31575-1-ming.qian@nxp.com (mailing list archive)
State New, archived
Headers show
Series media: amphion: handle firmware debug message | expand

Commit Message

Ming Qian Aug. 22, 2023, 5:52 a.m. UTC 
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>
---
 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 +++++++++
 4 files changed, 12 insertions(+)

Comments

Nicolas Dufresne Aug. 22, 2023, 7:48 p.m. UTC | #1 
Hi,

Le mardi 22 août 2023 à 13:52 +0800, Ming Qian a écrit :
> 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>
> ---
>  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 +++++++++
>  4 files changed, 12 insertions(+)
> 
> 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},
>  	{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
>  	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))
> +		dev_info(inst->dev, "instance %d firmware dbg msg : %s\n", inst->id, str);

I'm always a bit worried when a string is assumed nul-terminated and it comes
from a third party software (firmware). Perhaps we force it to terminate
somewhere already ? With this being verified (or corrected):

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> +}
> +
>  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 +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)
Ming Qian Aug. 23, 2023, 1:34 a.m. UTC | #2 
>Hi,
>
>Le mardi 22 août 2023 à 13:52 +0800, Ming Qian a écrit :
>> 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>
>> ---
>>  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 +++++++++
>>  4 files changed, 12 insertions(+)
>>
>> 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},
>>       {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
>>       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))
>> +             dev_info(inst->dev, "instance %d firmware dbg msg :
>> + %s\n", inst->id, str);
>
>I'm always a bit worried when a string is assumed nul-terminated and it
>comes from a third party software (firmware). Perhaps we force it to
>terminate somewhere already ? With this being verified (or corrected):
>
>Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>

Hi Nicolas,
    thanks for your reminder, I will force it to terminate at the end of the array in v2 patch.
Ming

>> +}
>> +
>>  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
>> +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 mbox series

Patch

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},
 	{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
 	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))
+		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 +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)