diff mbox series

[v4,2/2] media: amphion: Add a frame flush mode for decoder

Message ID 20250328064819.784-2-ming.qian@oss.nxp.com (mailing list archive)
State New
Headers show
Series [v4,1/2] media: amphion: Reduce decoding latency for HEVC decoder | expand

Commit Message

Ming Qian(OSS) March 28, 2025, 6:48 a.m. UTC 
From: Ming Qian <ming.qian@oss.nxp.com>

By default the amphion decoder will pre-parse 3 frames before starting
to decode the first frame. Alternatively, a block of flush padding data
can be appended to the frame, which will ensure that the decoder can
start decoding immediately after parsing the flush padding data, thus
potentially reducing decoding latency.

This mode was previously only enabled, when the display delay was set to
0. Allow the user to manually toggle the use of that mode via a module
parameter called low_latency, which enables the mode without
changing the display order.

Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
v4
- Improve the comment expressing
v3
- Improve commit message as recommended
- Add some comments to avoid code looks cryptic

 drivers/media/platform/amphion/vpu_malone.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Frank Li March 28, 2025, 2:26 p.m. UTC | #1 
On Fri, Mar 28, 2025 at 02:48:17PM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> By default the amphion decoder will pre-parse 3 frames before starting
> to decode the first frame. Alternatively, a block of flush padding data
> can be appended to the frame, which will ensure that the decoder can
> start decoding immediately after parsing the flush padding data, thus
> potentially reducing decoding latency.
>
> This mode was previously only enabled, when the display delay was set to
> 0. Allow the user to manually toggle the use of that mode via a module
> parameter called low_latency, which enables the mode without
> changing the display order.
>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
> v4
> - Improve the comment expressing
> v3
> - Improve commit message as recommended
> - Add some comments to avoid code looks cryptic
>
>  drivers/media/platform/amphion/vpu_malone.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/amphion/vpu_malone.c b/drivers/media/platform/amphion/vpu_malone.c
> index 88f8c16a451e..7f6251f7becb 100644
> --- a/drivers/media/platform/amphion/vpu_malone.c
> +++ b/drivers/media/platform/amphion/vpu_malone.c
> @@ -25,6 +25,10 @@
>  #include "vpu_imx8q.h"
>  #include "vpu_malone.h"
>
> +static bool low_latency;
> +module_param(low_latency, bool, 0644);
> +MODULE_PARM_DESC(low_latency, "Set low latency frame flush mode: 0 (disable) or 1 (enable)");
> +

If there are two malone instances, it will impact both instances, is it what
your expected? Prefer use sys interface to controller it.

Frank

>  #define CMD_SIZE			25600
>  #define MSG_SIZE			25600
>  #define CODEC_SIZE			0x1000
> @@ -1581,7 +1585,15 @@ static int vpu_malone_input_frame_data(struct vpu_malone_str_buffer __iomem *str
>
>  	vpu_malone_update_wptr(str_buf, wptr);
>
> -	if (disp_imm && !vpu_vb_is_codecconfig(vbuf)) {
> +	/*
> +	 * Enable the low latency flush mode if display delay is set to 0
> +	 * or the low latency frame flush mode if it is set to 1.
> +	 * The low latency flush mode requires some padding data to be appended to each frame,
> +	 * but there must not be any padding data between the sequence header and the frame.
> +	 * This module is currently only supported for the H264 and HEVC formats,
> +	 * for other formats, vpu_malone_add_scode() will return 0.
> +	 */
> +	if ((disp_imm || low_latency) && !vpu_vb_is_codecconfig(vbuf)) {
>  		ret = vpu_malone_add_scode(inst->core->iface,
>  					   inst->id,
>  					   &inst->stream_buffer,
> --
> 2.43.0-rc1
>
Ming Qian(OSS) March 31, 2025, 2:37 a.m. UTC | #2 
Hi Frank,

On 2025/3/28 22:26, Frank Li wrote:
> On Fri, Mar 28, 2025 at 02:48:17PM +0800, ming.qian@oss.nxp.com wrote:
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> By default the amphion decoder will pre-parse 3 frames before starting
>> to decode the first frame. Alternatively, a block of flush padding data
>> can be appended to the frame, which will ensure that the decoder can
>> start decoding immediately after parsing the flush padding data, thus
>> potentially reducing decoding latency.
>>
>> This mode was previously only enabled, when the display delay was set to
>> 0. Allow the user to manually toggle the use of that mode via a module
>> parameter called low_latency, which enables the mode without
>> changing the display order.
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>> ---
>> v4
>> - Improve the comment expressing
>> v3
>> - Improve commit message as recommended
>> - Add some comments to avoid code looks cryptic
>>
>>   drivers/media/platform/amphion/vpu_malone.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/amphion/vpu_malone.c b/drivers/media/platform/amphion/vpu_malone.c
>> index 88f8c16a451e..7f6251f7becb 100644
>> --- a/drivers/media/platform/amphion/vpu_malone.c
>> +++ b/drivers/media/platform/amphion/vpu_malone.c
>> @@ -25,6 +25,10 @@
>>   #include "vpu_imx8q.h"
>>   #include "vpu_malone.h"
>>
>> +static bool low_latency;
>> +module_param(low_latency, bool, 0644);
>> +MODULE_PARM_DESC(low_latency, "Set low latency frame flush mode: 0 (disable) or 1 (enable)");
>> +
> 
> If there are two malone instances, it will impact both instances, is it what
> your expected? Prefer use sys interface to controller it.
> 
> Frank

Yes, that's we expected.
I prefer to say that we use this parameter to adjust the behavior of the
firmware. I thought about enabling this mode by default, but our team
prefers to add a parameter mode, but keep the previous behavior.and then
enable it as needed.

Thanks,
Ming

> 
>>   #define CMD_SIZE			25600
>>   #define MSG_SIZE			25600
>>   #define CODEC_SIZE			0x1000
>> @@ -1581,7 +1585,15 @@ static int vpu_malone_input_frame_data(struct vpu_malone_str_buffer __iomem *str
>>
>>   	vpu_malone_update_wptr(str_buf, wptr);
>>
>> -	if (disp_imm && !vpu_vb_is_codecconfig(vbuf)) {
>> +	/*
>> +	 * Enable the low latency flush mode if display delay is set to 0
>> +	 * or the low latency frame flush mode if it is set to 1.
>> +	 * The low latency flush mode requires some padding data to be appended to each frame,
>> +	 * but there must not be any padding data between the sequence header and the frame.
>> +	 * This module is currently only supported for the H264 and HEVC formats,
>> +	 * for other formats, vpu_malone_add_scode() will return 0.
>> +	 */
>> +	if ((disp_imm || low_latency) && !vpu_vb_is_codecconfig(vbuf)) {
>>   		ret = vpu_malone_add_scode(inst->core->iface,
>>   					   inst->id,
>>   					   &inst->stream_buffer,
>> --
>> 2.43.0-rc1
>>
diff mbox series

Patch

diff --git a/drivers/media/platform/amphion/vpu_malone.c b/drivers/media/platform/amphion/vpu_malone.c
index 88f8c16a451e..7f6251f7becb 100644
--- a/drivers/media/platform/amphion/vpu_malone.c
+++ b/drivers/media/platform/amphion/vpu_malone.c
@@ -25,6 +25,10 @@ 
 #include "vpu_imx8q.h"
 #include "vpu_malone.h"
 
+static bool low_latency;
+module_param(low_latency, bool, 0644);
+MODULE_PARM_DESC(low_latency, "Set low latency frame flush mode: 0 (disable) or 1 (enable)");
+
 #define CMD_SIZE			25600
 #define MSG_SIZE			25600
 #define CODEC_SIZE			0x1000
@@ -1581,7 +1585,15 @@  static int vpu_malone_input_frame_data(struct vpu_malone_str_buffer __iomem *str
 
 	vpu_malone_update_wptr(str_buf, wptr);
 
-	if (disp_imm && !vpu_vb_is_codecconfig(vbuf)) {
+	/*
+	 * Enable the low latency flush mode if display delay is set to 0
+	 * or the low latency frame flush mode if it is set to 1.
+	 * The low latency flush mode requires some padding data to be appended to each frame,
+	 * but there must not be any padding data between the sequence header and the frame.
+	 * This module is currently only supported for the H264 and HEVC formats,
+	 * for other formats, vpu_malone_add_scode() will return 0.
+	 */
+	if ((disp_imm || low_latency) && !vpu_vb_is_codecconfig(vbuf)) {
 		ret = vpu_malone_add_scode(inst->core->iface,
 					   inst->id,
 					   &inst->stream_buffer,