media: venus: Update to bitrate based clock scaling
Subsystems (4)
PATCH 1/1 media: venus: Update to bitrate based clock scaling
This patch introduces bitrate based clock scaling. Also, clock scaling is now triggered before buffer being queued to the device. This checks for frequency requirement throughout the session and updates clock with correct frequency only if requirement is changed.
Aniket Masule (1):
media: venus: Update to bitrate based clock scaling
drivers/media/platform/qcom/venus/core.c | 16 +++++------ drivers/media/platform/qcom/venus/core.h | 1 + drivers/media/platform/qcom/venus/helpers.c | 43 +++++++++++++++++++++++++---- 3 files changed, 47 insertions(+), 13 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 1/1 media: venus: Update to bitrate based clock scaling
Introduced clock scaling using bitrate, current
calculations consider only the cycles per mb.
Also, clock scaling is now triggered before every
buffer being queued to the device. This helps in
deciding precise clock cycles required.
--- drivers/media/platform/qcom/venus/core.c | 16 +++++------ drivers/media/platform/qcom/venus/core.h | 1 + drivers/media/platform/qcom/venus/helpers.c | 43 +++++++++++++++++++++++++---- 3 files changed, 47 insertions(+), 13 deletions(-)
drivers/media/platform/qcom/venus/core.c +8 -8
@@ -474,14 +474,14 @@ static __maybe_unused int venus_runtime_resume(struct device *dev) };static struct codec_freq_data sdm845_codec_freq_data[] = { - {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_MPEG2 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_VP9 ,VIDC_SESSION_TYPE_DEC ,200 }, + {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_MPEG2 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_VP9 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, };static const struct venus_resources sdm845_res = {
drivers/media/platform/qcom/venus/core.h +1 -0
@@ -39,6 +39,7 @@ struct codec_freq_data {u32 pixfmt;u32 session_type;unsigned int vpp_freq; +unsigned int vsp_freq; };struct venus_resources {
drivers/media/platform/qcom/venus/helpers.c +38 -5
87 lines not on this page · raw message
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Aniket,
Quoted 45 lines · Aniket Masule
Introduced clock scaling using bitrate, current calculations consider only the cycles per mb. Also, clock scaling is now triggered before every buffer being queued to the device. This helps in deciding precise clock cycles required.
Signed-off-by Aniket Masule <[email protected]>--- drivers/media/platform/qcom/venus/core.c | 16 +++++------ drivers/media/platform/qcom/venus/core.h | 1 + drivers/media/platform/qcom/venus/helpers.c | 43 +++++++++++++++++++++++++---- 3 files changed, 47 insertions(+), 13 deletions(-)drivers/media/platform/qcom/venus/core.c +8
-8@@ -474,14 +474,14 @@ static __maybe_unused int venus_runtime_resume(struct device *dev) };static struct codec_freq_data sdm845_codec_freq_data[] = { - {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_MPEG2 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_VP9 ,VIDC_SESSION_TYPE_DEC ,200 }, + {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_MPEG2 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_VP9 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, };static const struct venus_resources sdm845_res = {drivers/media/platform/qcom/venus/core.h +0
-0
drivers/media/platform/qcom/venus/core.h struct codec_freq_data { · line 42
Quoted 4 lines
drivers/media/platform/qcom/venus/core.h +1
-0@@ -39,6 +39,7 @@ struct codec_freq_data {u32 pixfmt;u32 session_type;unsigned int vpp_freq; +unsigned int vsp_freq;
unsigned long?
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks(struct venus_inst *inst) · line 383
Quoted 13 lines
drivers/media/platform/qcom/venus/core.h +0
-0};struct venus_resources {drivers/media/platform/qcom/venus/helpers.c +2
-0@@ -379,6 +379,9 @@ static int scale_clocks(struct venus_inst *inst)unsigned int i;int ret; +if (inst->state ==INST_START ) +return 0 ;
This condition is related (probably) to the change in venus_helper_vb2_buf_queue() but shouldn't it be copied in scale_clocks_v4 too?
Quoted 12 lines
drivers/media/platform/qcom/venus/helpers.c +3
-1+ mbs_per_sec =load_per_type (core,VIDC_SESSION_TYPE_ENC ) +load_per_type (core,VIDC_SESSION_TYPE_DEC );@@ -418,17 +421,26 @@ static int scale_clocks(struct venus_inst *inst)return ret; } -static unsigned long calculate_vpp_freq (struct venus_inst *inst) +static unsigned long calculate_inst_freq (struct venus_inst *inst, +unsigned long filled_len) {
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks(struct venus_inst *inst) · line 435
Quoted 10 lines
drivers/media/platform/qcom/venus/helpers.c +3
-1-unsigned long vpp_freq =0 ; +unsigned long vpp_freq =0 , vsp_freq =0 ; +u64 fps = inst->fps;u32 mbs_per_sec; mbs_per_sec =load_per_instance (inst); vpp_freq = mbs_per_sec * inst->clk_data.codec_freq_data->vpp_freq;/* 21 / 20 is overhead factor */ vpp_freq += vpp_freq /20 ; + vsp_freq = mbs_per_sec * inst->clk_data.codec_freq_data->vsp_freq;
this calculation is not used below. Is that intentional?
Quoted 13 lines
drivers/media/platform/qcom/venus/helpers.c +7
-1+ +/* 10 / 7 is overhead factor */ +if (inst->session_type ==VIDC_SESSION_TYPE_ENC ) + vsp_freq = (inst->controls.enc.bitrate *10 ) /7 ; +else + vsp_freq = ((fps * filled_len *8 ) *10 ) /7 ; -return vpp_freq; +return max (vpp_freq, vsp_freq); }static int scale_clocks_v4 (struct venus_inst *inst)
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks_v4(struct venus_inst *inst) · line 454
Quoted 8 lines
drivers/media/platform/qcom/venus/helpers.c +2
-1@@ -436,14 +448,30 @@ static int scale_clocks_v4(struct venus_inst *inst)struct venus_core *core = inst->core;const struct freq_tbl *table = core->res->freq_tbl;unsigned int num_rows = core->res->freq_tbl_size; - +struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;struct clk *clk = core->clks[0 ];struct device *dev = core->dev; +
drop this addition of blank line.
Quoted 7 lines
drivers/media/platform/qcom/venus/helpers.c +3
-0unsigned int i;unsigned long freq =0 , freq_core0 =0 , freq_core1 =0 ; +unsigned long filled_len =0 ; +struct venus_buffer *buf, *n; +struct vb2_buffer *vb;int ret;
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks_v4(struct venus_inst *inst) · line 470
Quoted 10 lines
drivers/media/platform/qcom/venus/helpers.c +9
-1- freq =calculate_vpp_freq (inst); +mutex_lock (&inst->lock); +v4l2_m2m_for_each_src_buf_safe (m2m_ctx, buf, n) { + vb = &buf->vb.vb2_buf; + filled_len =max (filled_len,vb2_get_plane_payload (vb,0 )); + } +mutex_unlock (&inst->lock); + +if (inst->session_type ==VIDC_SESSION_TYPE_DEC && !filled_len) { +dev_dbg (dev,"%s: No input to the session\n" , __func__);
please drop this debug message, if the user pushes empty buffers something will blow up earlier.
Quoted 8 lines
drivers/media/platform/qcom/venus/helpers.c +4
-0+return 0 ; + } + + freq =calculate_inst_freq (inst, filled_len);if (freq > table[0 ].freq) goto err;
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks_v4(struct venus_inst *inst) · line 504
Quoted 6 lines
drivers/media/platform/qcom/venus/helpers.c +3
-0@@ -471,6 +499,9 @@ static int scale_clocks_v4(struct venus_inst *inst) freq =max (freq_core0, freq_core1); +if (clk_get_rate (clk) == freq) +return 0 ; +
above check is included in clk_set_rate(), you don't need it.
Quoted 13 lines
drivers/media/platform/qcom/venus/helpers.c +2
-0ret =clk_set_rate (clk, freq);if (ret) goto err;@@ -1150,6 +1181,8 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)if (ret) goto unlock; +load_scale_clocks (inst); + ret =session_process_buf (inst, vbuf);if (ret)return_buf_error (inst, vbuf);
Signature · 2 lines
regards, Stan
Hi Stan,
Quoted 44 lines · Stanimir Varbanov
Hi Aniket,
Quoted 41 lines · Aniket Masule
Introduced clock scaling using bitrate, current calculations consider only the cycles per mb. Also, clock scaling is now triggered before every buffer being queued to the device. This helps in deciding precise clock cycles required.
Signed-off-by Aniket Masule <[email protected]>--- drivers/media/platform/qcom/venus/core.c | 16 +++++------ drivers/media/platform/qcom/venus/core.h | 1 + drivers/media/platform/qcom/venus/helpers.c | 43 +++++++++++++++++++++++++---- 3 files changed, 47 insertions(+), 13 deletions(-)drivers/media/platform/qcom/venus/core.c +8
-8@@ -474,14 +474,14 @@ static __maybe_unused int venus_runtime_resume(struct device *dev) };static struct codec_freq_data sdm845_codec_freq_data[] = { - {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_ENC ,675 }, - {V4L2_PIX_FMT_MPEG2 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_DEC ,200 }, - {V4L2_PIX_FMT_VP9 ,VIDC_SESSION_TYPE_DEC ,200 }, + {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_ENC ,675 ,10 }, + {V4L2_PIX_FMT_MPEG2 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_H264 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_HEVC ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_VP8 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, + {V4L2_PIX_FMT_VP9 ,VIDC_SESSION_TYPE_DEC ,200 ,10 }, };
Quoted 13 lines
drivers/media/platform/qcom/venus/core.h struct codec_freq_data { · line 42
Quoted 10 lines
static const struct
venus_resourcessdm845_res= {drivers/media/platform/qcom/venus/core.h +1
-0@@ -39,6 +39,7 @@ struct codec_freq_data {u32 pixfmt;u32 session_type;unsigned int vpp_freq; +unsigned int vsp_freq;unsigned long?
The hard-coded values for both vpp and vsp will in few hundreds. So, unsigned int would work fine.
Quoted 11 lines
Quoted 11 lines
drivers/media/platform/qcom/venus/core.h +0
-0};struct venus_resources {drivers/media/platform/qcom/venus/helpers.c +0
-0@@ -379,6 +379,9 @@ static int scale_clocks(struct venus_inst *inst)unsigned int i;int ret;
Quoted 7 lines
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks(struct venus_inst *inst) · line 383
Quoted 2 lines
drivers/media/platform/qcom/venus/helpers.c +2
-0+if (inst->state ==INST_START ) +return 0 ;This condition is related (probably) to the change in
venus_helper_vb2_buf_queue()but shouldn't it be copied inscale_clocks_v4too?
Yes, this is related to change in venus_helper_vb2_buf_queue. The intention is trigger clock scaling only in case of v4 whenever instance is in INST_START state. This would call scale_clocks only when instance is in INST_INIT state, which alignment to earlier call flow.
Quoted 17 lines
Quoted 17 lines
drivers/media/platform/qcom/venus/helpers.c +5
-2+ mbs_per_sec =load_per_type (core,VIDC_SESSION_TYPE_ENC ) +load_per_type (core,VIDC_SESSION_TYPE_DEC );@@ -418,17 +421,26 @@ static int scale_clocks(struct venus_inst *inst)return ret; } -static unsigned long calculate_vpp_freq (struct venus_inst *inst) +static unsigned long calculate_inst_freq (struct venus_inst *inst, +unsigned long filled_len) { -unsigned long vpp_freq =0 ; +unsigned long vpp_freq =0 , vsp_freq =0 ; +u64 fps = inst->fps;u32 mbs_per_sec;
Quoted 8 lines
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks(struct venus_inst *inst) · line 435
Quoted 5 lines
drivers/media/platform/qcom/venus/helpers.c +1
-0mbs_per_sec =load_per_instance (inst); vpp_freq = mbs_per_sec * inst->clk_data.codec_freq_data->vpp_freq;/* 21 / 20 is overhead factor */ vpp_freq += vpp_freq /20 ; + vsp_freq = mbs_per_sec * inst->clk_data.codec_freq_data->vsp_freq;this calculation is not used below. Is that intentional?
I will refine this calculations, need to add bitrate with above calculated vsp cycles.
Quoted 32 lines
Quoted 13 lines
drivers/media/platform/qcom/venus/helpers.c +7
-1+ +/* 10 / 7 is overhead factor */ +if (inst->session_type ==VIDC_SESSION_TYPE_ENC ) + vsp_freq = (inst->controls.enc.bitrate *10 ) /7 ; +else + vsp_freq = ((fps * filled_len *8 ) *10 ) /7 ; -return vpp_freq; +return max (vpp_freq, vsp_freq); }static int scale_clocks_v4 (struct venus_inst *inst)drivers/media/platform/qcom/venus/helpers.c static int scale_clocks_v4(struct venus_inst · line 455
Quoted 9 lines
drivers/media/platform/qcom/venus/helpers.c +2
-1@@ -436,14 +448,30 @@ static int scale_clocks_v4(struct venus_inst *inst)struct venus_core *core = inst->core;const struct freq_tbl *table = core->res->freq_tbl;unsigned int num_rows = core->res->freq_tbl_size; - +struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;struct clk *clk = core->clks[0 ];struct device *dev = core->dev; +drop this addition of blank line.
Quoted 7 lines
drivers/media/platform/qcom/venus/helpers.c +3
-0unsigned int i;unsigned long freq =0 , freq_core0 =0 , freq_core1 =0 ; +unsigned long filled_len =0 ; +struct venus_buffer *buf, *n; +struct vb2_buffer *vb;int ret;
Quoted 14 lines
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks_v4(struct venus_inst *inst) · line 470
Quoted 10 lines
drivers/media/platform/qcom/venus/helpers.c +9
-1- freq =calculate_vpp_freq (inst); +mutex_lock (&inst->lock); +v4l2_m2m_for_each_src_buf_safe (m2m_ctx, buf, n) { + vb = &buf->vb.vb2_buf; + filled_len =max (filled_len,vb2_get_plane_payload (vb,0 )); + } +mutex_unlock (&inst->lock); + +if (inst->session_type ==VIDC_SESSION_TYPE_DEC && !filled_len) { +dev_dbg (dev,"%s: No input to the session\n" , __func__);please drop this debug message, if the user pushes empty buffers something will blow up earlier.
Yes.
Quoted 10 lines
Quoted 10 lines
drivers/media/platform/qcom/venus/helpers.c +4
-0+return 0 ; + } + + freq =calculate_inst_freq (inst, filled_len);if (freq > table[0 ].freq)drivers/media/platform/qcom/venus/helpers.c +0
-0goto err;@@ -471,6 +499,9 @@ static int scale_clocks_v4(struct venus_inst *inst)
Quoted 8 lines
drivers/media/platform/qcom/venus/helpers.c static int scale_clocks_v4(struct venus_inst *inst) · line 504
Quoted 5 lines
drivers/media/platform/qcom/venus/helpers.c +3
-0freq =max (freq_core0, freq_core1); +if (clk_get_rate (clk) == freq) +return 0 ; +above check is included in
clk_set_rate(), you don't need it.
Yes.
Quoted 8 lines
Quoted 8 lines
drivers/media/platform/qcom/venus/helpers.c +0
-0ret =clk_set_rate (clk, freq);if (ret)drivers/media/platform/qcom/venus/helpers.c +0
-0goto err;@@ -1150,6 +1181,8 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)if (ret) goto unlock;
Quoted 6 lines
Quoted 6 lines
drivers/media/platform/qcom/venus/helpers.c +2
-0+load_scale_clocks (inst); + ret =session_process_buf (inst, vbuf);if (ret)
return_buf_error(inst, vbuf);
Regards,
Aniket
Hi Aniket,
Quoted 7 lines · Aniket Masule
This patch introduces bitrate based clock scaling. Also, clock scaling is now triggered before buffer being queued to the device. This checks for frequency requirement throughout the session and updates clock with correct frequency only if requirement is changed.
Aniket Masule (1):
media: venus: Update to bitrate based clock scaling
You don't need cover letter for one patch. Also, please include this patch in "venus: Update clock scaling and core selection" series.
Quoted 6 lines
drivers/media/platform/qcom/venus/core.c | 16 +++++------ drivers/media/platform/qcom/venus/core.h | 1 + drivers/media/platform/qcom/venus/helpers.c | 43 +++++++++++++++++++++++++---- 3 files changed, 47 insertions(+), 13 deletions(-)
Signature · 2 lines
regards, Stan