* [PATCH] media: venus: Update to bitrate based clock scaling
@ 2019-06-26 8:23 Aniket Masule
2019-06-26 8:23 ` Aniket Masule
2019-07-01 15:29 ` Stanimir Varbanov
0 siblings, 2 replies; 5+ messages in thread
From: Aniket Masule @ 2019-06-26 8:23 UTC (permalink / raw)
To: linux-media, stanimir.varbanov
Cc: linux-kernel, linux-arm-msm, vgarodia, 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
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(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] media: venus: Update to bitrate based clock scaling
2019-06-26 8:23 [PATCH] media: venus: Update to bitrate based clock scaling Aniket Masule
@ 2019-06-26 8:23 ` Aniket Masule
2019-07-01 15:45 ` Stanimir Varbanov
2019-07-01 15:29 ` Stanimir Varbanov
1 sibling, 1 reply; 5+ messages in thread
From: Aniket Masule @ 2019-06-26 8:23 UTC (permalink / raw)
To: linux-media, stanimir.varbanov
Cc: linux-kernel, linux-arm-msm, vgarodia, 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 <amasule@codeaurora.org>
---
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(-)
diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index f1597d6..ad6bb74 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -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 = {
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 2ed6496..b964b7c 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -39,6 +39,7 @@ struct codec_freq_data {
u32 pixfmt;
u32 session_type;
unsigned int vpp_freq;
+ unsigned int vsp_freq;
};
struct venus_resources {
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index ef35fd8..634778a 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -379,6 +379,9 @@ static int scale_clocks(struct venus_inst *inst)
unsigned int i;
int ret;
+ if (inst->state == INST_START)
+ return 0;
+
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;
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;
+
+ /* 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)
@@ -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;
+
unsigned 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;
- 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__);
+ return 0;
+ }
+
+ freq = calculate_inst_freq(inst, filled_len);
if (freq > table[0].freq)
goto err;
@@ -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;
+
ret = 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);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] media: venus: Update to bitrate based clock scaling
2019-06-26 8:23 [PATCH] media: venus: Update to bitrate based clock scaling Aniket Masule
2019-06-26 8:23 ` Aniket Masule
@ 2019-07-01 15:29 ` Stanimir Varbanov
1 sibling, 0 replies; 5+ messages in thread
From: Stanimir Varbanov @ 2019-07-01 15:29 UTC (permalink / raw)
To: Aniket Masule, linux-media; +Cc: linux-kernel, linux-arm-msm, vgarodia
Hi Aniket,
On 6/26/19 11:23 AM, Aniket Masule wrote:
> 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.
>
> 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(-)
>
--
regards,
Stan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: venus: Update to bitrate based clock scaling
2019-06-26 8:23 ` Aniket Masule
@ 2019-07-01 15:45 ` Stanimir Varbanov
2019-07-02 5:37 ` amasule
0 siblings, 1 reply; 5+ messages in thread
From: Stanimir Varbanov @ 2019-07-01 15:45 UTC (permalink / raw)
To: Aniket Masule, linux-media; +Cc: linux-kernel, linux-arm-msm, vgarodia
Hi Aniket,
On 6/26/19 11:23 AM, Aniket Masule wrote:
> 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 <amasule@codeaurora.org>
> ---
> 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(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index f1597d6..ad6bb74 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -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 = {
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 2ed6496..b964b7c 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -39,6 +39,7 @@ struct codec_freq_data {
> u32 pixfmt;
> u32 session_type;
> unsigned int vpp_freq;
> + unsigned int vsp_freq;
unsigned long?
> };
>
> struct venus_resources {
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index ef35fd8..634778a 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -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?
> +
> 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;
>
> 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?
> +
> + /* 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)
> @@ -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.
> unsigned 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;
>
> - 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.
> + return 0;
> + }
> +
> + freq = calculate_inst_freq(inst, filled_len);
>
> if (freq > table[0].freq)
> goto err;
> @@ -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.
> ret = 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);
>
--
regards,
Stan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: venus: Update to bitrate based clock scaling
2019-07-01 15:45 ` Stanimir Varbanov
@ 2019-07-02 5:37 ` amasule
0 siblings, 0 replies; 5+ messages in thread
From: amasule @ 2019-07-02 5:37 UTC (permalink / raw)
To: Stanimir Varbanov
Cc: linux-media, linux-kernel, linux-arm-msm, vgarodia, linux-arm-msm-owner
Hi Stan,
On 2019-07-01 21:15, Stanimir Varbanov wrote:
> Hi Aniket,
>
> On 6/26/19 11:23 AM, Aniket Masule wrote:
>> 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 <amasule@codeaurora.org>
>> ---
>> 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(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.c
>> b/drivers/media/platform/qcom/venus/core.c
>> index f1597d6..ad6bb74 100644
>> --- a/drivers/media/platform/qcom/venus/core.c
>> +++ b/drivers/media/platform/qcom/venus/core.c
>> @@ -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 = {
>> diff --git a/drivers/media/platform/qcom/venus/core.h
>> b/drivers/media/platform/qcom/venus/core.h
>> index 2ed6496..b964b7c 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -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.
>> };
>>
>> struct venus_resources {
>> diff --git a/drivers/media/platform/qcom/venus/helpers.c
>> b/drivers/media/platform/qcom/venus/helpers.c
>> index ef35fd8..634778a 100644
>> --- a/drivers/media/platform/qcom/venus/helpers.c
>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>> @@ -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?
>
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.
>> +
>> 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;
>>
>> 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?
>
I will refine this calculations, need to add bitrate with above
calculated vsp cycles.
>> +
>> + /* 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)
>> @@ -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.
>
>> unsigned 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;
>>
>> - 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.
>> + return 0;
>> + }
>> +
>> + freq = calculate_inst_freq(inst, filled_len);
>>
>> if (freq > table[0].freq)
>> goto err;
>> @@ -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.
>
Yes.
>> ret = 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);
>>
Regards,
Aniket
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-02 5:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-26 8:23 [PATCH] media: venus: Update to bitrate based clock scaling Aniket Masule
2019-06-26 8:23 ` Aniket Masule
2019-07-01 15:45 ` Stanimir Varbanov
2019-07-02 5:37 ` amasule
2019-07-01 15:29 ` Stanimir Varbanov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).