This series includes the changes to
- add V4L2_PIX_FMT_P010 as supported decoder format.
- consider dpb color format while calculating buffer
size for dpb buffers.
- update dpb and opb color format when bit depth
changes is detected, also update preferred color
format to P010 in this case.
With this series, divided the previous version [1] into
multiple patches as suggested in review comments.
[1] https://patchwork.linuxtv.org/project/linux-media/list/?series=10376
Dikshita Agarwal (4):
venus: add support for V4L2_PIX_FMT_P010 color format
venus: update calculation for dpb buffers
venus: add handling of bit depth change from firmwar
venus: return P010 as preferred format for 10 bit decode
drivers/media/platform/qcom/venus/helpers.c | 24 ++++++++++++++++++++++
drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++
.../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++-
drivers/media/platform/qcom/venus/vdec.c | 16 +++++++++++++--
4 files changed, 48 insertions(+), 3 deletions(-)
--
2.7.4
Set opb format to TP10_UWC and dpb to client set format
when bit depth change to 10 bit is detecting by firmware.
Signed-off-by: Dikshita Agarwal <[email protected]>
---
drivers/media/platform/qcom/venus/helpers.c | 18 ++++++++++++++++++
drivers/media/platform/qcom/venus/vdec.c | 5 ++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 4ad6232..4f48ebd 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1770,6 +1770,24 @@ int venus_helper_get_out_fmts(struct venus_inst *inst, u32 v4l2_fmt,
if (!caps)
return -EINVAL;
+ if (inst->bit_depth == VIDC_BITDEPTH_10 &&
+ inst->session_type == VIDC_SESSION_TYPE_DEC) {
+ found_ubwc = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT,
+ HFI_COLOR_FORMAT_YUV420_TP10_UBWC);
+ found = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT2,
+ fmt);
+ if (found_ubwc && found) {
+ /*
+ * Hard-code DPB buffers to be 10bit UBWC
+ * until V4L2 is able to expose compressed/tiled
+ * formats to applications.
+ */
+ *out_fmt = HFI_COLOR_FORMAT_YUV420_TP10_UBWC;
+ *out2_fmt = fmt;
+ return 0;
+ }
+ }
+
if (ubwc) {
ubwc_fmt = fmt | HFI_COLOR_FORMAT_UBWC_BASE;
found_ubwc = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT,
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 687d62e..69f7f6e 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -701,6 +701,9 @@ static int vdec_set_work_route(struct venus_inst *inst)
}
#define is_ubwc_fmt(fmt) (!!((fmt) & HFI_COLOR_FORMAT_UBWC_BASE))
+#define is_10bit_ubwc_fmt(fmt) (!!((fmt) & HFI_COLOR_FORMAT_10_BIT_BASE & \
+ HFI_COLOR_FORMAT_UBWC_BASE))
+
static int vdec_output_conf(struct venus_inst *inst)
{
@@ -748,7 +751,7 @@ static int vdec_output_conf(struct venus_inst *inst)
inst->opb_fmt = out2_fmt;
inst->dpb_buftype = HFI_BUFFER_OUTPUT;
inst->dpb_fmt = out_fmt;
- } else if (is_ubwc_fmt(out2_fmt)) {
+ } else if (is_ubwc_fmt(out2_fmt) || is_10bit_ubwc_fmt(out_fmt)) {
inst->opb_buftype = HFI_BUFFER_OUTPUT;
inst->opb_fmt = out_fmt;
inst->dpb_buftype = HFI_BUFFER_OUTPUT2;
--
2.7.4
If bit depth is detected as 10 bit by firmware, return
P010 as preferred decoder format to the client.
Signed-off-by: Dikshita Agarwal <[email protected]>
---
drivers/media/platform/qcom/venus/vdec.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 69f7f6e..ed11dc2 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -1468,8 +1468,13 @@ static void vdec_event_change(struct venus_inst *inst,
inst->out_width = ev_data->width;
inst->out_height = ev_data->height;
- if (inst->bit_depth != ev_data->bit_depth)
+ if (inst->bit_depth != ev_data->bit_depth) {
inst->bit_depth = ev_data->bit_depth;
+ if (inst->bit_depth == VIDC_BITDEPTH_10)
+ inst->fmt_cap = &vdec_formats[3];
+ else
+ inst->fmt_cap = &vdec_formats[0];
+ }
if (inst->pic_struct != ev_data->pic_struct)
inst->pic_struct = ev_data->pic_struct;
--
2.7.4
Use dpb color format, width and height of output port
for calculating buffer size of dpb buffers.
Signed-off-by: Dikshita Agarwal <[email protected]>
---
drivers/media/platform/qcom/venus/helpers.c | 4 ++++
drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++
drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++-
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 5946def..4ad6232 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -641,12 +641,16 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype,
if (is_dec) {
params.width = inst->width;
params.height = inst->height;
+ params.out_width = inst->out_width;
+ params.out_height = inst->out_height;
params.codec = inst->fmt_out->pixfmt;
params.hfi_color_fmt = to_hfi_raw_fmt(inst->fmt_cap->pixfmt);
params.dec.max_mbs_per_frame = mbs_per_frame_max(inst);
params.dec.buffer_size_limit = 0;
params.dec.is_secondary_output =
inst->opb_buftype == HFI_BUFFER_OUTPUT2;
+ if (params.dec.is_secondary_output)
+ params.hfi_dpb_color_fmt = inst->dpb_fmt;
params.dec.is_interlaced =
inst->pic_struct != HFI_INTERLACE_FRAME_PROGRESSIVE;
} else {
diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
index 52a51a3..25e6074 100644
--- a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
+++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
@@ -12,8 +12,11 @@
struct hfi_plat_buffers_params {
u32 width;
u32 height;
+ u32 out_width;
+ u32 out_height;
u32 codec;
u32 hfi_color_fmt;
+ u32 hfi_dpb_color_fmt;
enum hfi_version version;
u32 num_vpp_pipes;
union {
diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
index ea25c45..3855b04 100644
--- a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
+++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
@@ -1185,6 +1185,7 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype,
enum hfi_version version = params->version;
u32 codec = params->codec;
u32 width = params->width, height = params->height, out_min_count;
+ u32 out_width = params->out_width, out_height = params->out_height;
struct dec_bufsize_ops *dec_ops;
bool is_secondary_output = params->dec.is_secondary_output;
bool is_interlaced = params->dec.is_interlaced;
@@ -1235,7 +1236,12 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype,
bufreq->count_min = out_min_count;
bufreq->size =
venus_helper_get_framesz_raw(params->hfi_color_fmt,
- width, height);
+ out_width, out_height);
+ if (buftype == HFI_BUFFER_OUTPUT &&
+ params->dec.is_secondary_output)
+ bufreq->size =
+ venus_helper_get_framesz_raw(params->hfi_dpb_color_fmt,
+ out_width, out_height);
} else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH(version)) {
bufreq->size = dec_ops->scratch(width, height, is_interlaced);
} else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH_1(version)) {
--
2.7.4
add V4L2_PIX_FMT_P010 as supported color format for decoder.
Signed-off-by: Dikshita Agarwal <[email protected]>
---
drivers/media/platform/qcom/venus/helpers.c | 2 ++
drivers/media/platform/qcom/venus/vdec.c | 4 ++++
2 files changed, 6 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index ab6a29f..5946def 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -612,6 +612,8 @@ static u32 to_hfi_raw_fmt(u32 v4l2_fmt)
return HFI_COLOR_FORMAT_NV12_UBWC;
case V4L2_PIX_FMT_QC10C:
return HFI_COLOR_FORMAT_YUV420_TP10_UBWC;
+ case V4L2_PIX_FMT_P010:
+ return HFI_COLOR_FORMAT_P010;
default:
break;
}
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 4ceaba3..687d62e 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -43,6 +43,10 @@ static const struct venus_format vdec_formats[] = {
.num_planes = 1,
.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
}, {
+ .pixfmt = V4L2_PIX_FMT_P010,
+ .num_planes = 1,
+ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+ }, {
.pixfmt = V4L2_PIX_FMT_MPEG4,
.num_planes = 1,
.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
--
2.7.4
On 04/05/2023 11:36, Dikshita Agarwal wrote:
> This series includes the changes to
> - add V4L2_PIX_FMT_P010 as supported decoder format.
> - consider dpb color format while calculating buffer
> size for dpb buffers.
> - update dpb and opb color format when bit depth
> changes is detected, also update preferred color
> format to P010 in this case.
>
> With this series, divided the previous version [1] into
> multiple patches as suggested in review comments.
>
> [1] https://patchwork.linuxtv.org/project/linux-media/list/?series=10376
>
> Dikshita Agarwal (4):
> venus: add support for V4L2_PIX_FMT_P010 color format
> venus: update calculation for dpb buffers
> venus: add handling of bit depth change from firmwar
> venus: return P010 as preferred format for 10 bit decode
>
> drivers/media/platform/qcom/venus/helpers.c | 24 ++++++++++++++++++++++
> drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++
> .../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++-
> drivers/media/platform/qcom/venus/vdec.c | 16 +++++++++++++--
> 4 files changed, 48 insertions(+), 3 deletions(-)
>
For future reference a series like this should:
1. Include a log of what changed between the last series and this
2. Describe which comments you addressed
I generally try to say
"Added newline to dts - Konrad"
"Sent the series as a -v3 - Bryan"
etc
3. Ideally provide a link to the previous series in
https://lore.kernel.org/linux-arm-msm/[email protected]/
4. Use versioning
This set should be prefixed with "v2-0000-cover-letter"
"v2-0001-add-support" etc
"git format-patch mybase..targettip --cover-letter -v2"
---
bod
On 04/05/2023 14:49, Bryan O'Donoghue wrote:
> On 04/05/2023 11:36, Dikshita Agarwal wrote:
>> This series includes the changes to
>> - add V4L2_PIX_FMT_P010 as supported decoder format.
>> - consider dpb color format while calculating buffer
>> size for dpb buffers.
>> - update dpb and opb color format when bit depth
>> changes is detected, also update preferred color
>> format to P010 in this case.
>>
>> With this series, divided the previous version [1] into
>> multiple patches as suggested in review comments.
>>
>> [1] https://patchwork.linuxtv.org/project/linux-media/list/?series=10376
>>
>> Dikshita Agarwal (4):
>> venus: add support for V4L2_PIX_FMT_P010 color format
>> venus: update calculation for dpb buffers
>> venus: add handling of bit depth change from firmwar
>> venus: return P010 as preferred format for 10 bit decode
>>
>> drivers/media/platform/qcom/venus/helpers.c | 24
>> ++++++++++++++++++++++
>> drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++
>> .../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++-
>> drivers/media/platform/qcom/venus/vdec.c | 16 +++++++++++++--
>> 4 files changed, 48 insertions(+), 3 deletions(-)
>>
>
> For future reference a series like this should:
>
> 1. Include a log of what changed between the last series and this
> 2. Describe which comments you addressed
> I generally try to say
> "Added newline to dts - Konrad"
> "Sent the series as a -v3 - Bryan"
> etc
> 3. Ideally provide a link to the previous series in
>
> https://lore.kernel.org/linux-arm-msm/[email protected]/
> 4. Use versioning
> This set should be prefixed with "v2-0000-cover-letter"
> "v2-0001-add-support" etc
>
> "git format-patch mybase..targettip --cover-letter -v2"
>
> ---
> bod
Doh I see you did most of that - just missed the V2.
Please remember to version your subsequent series. "git format-patch -v2"
---
bod
On 04/05/2023 15:04, Vikash Garodia wrote:
>> Doh I see you did most of that - just missed the V2.
>>
>> Please remember to version your subsequent series. "git format-patch -v2"
>
> Does this qualify for a version upgrade when a single patch is
> subsequently raised as series ? IMO, the link
>
> to previous single patch in cover letter and then starting the series
> (as v0) seems to provide the required info.
Hmm. I'd say any series should have an increment in it to differentiate,
with the exception being RESEND.
Also you are splitting one patch into four.
Looking through a bunch of email it might be not immediately obvious to
understand that the new series and old series differ, which is IMO how
the version numbers help others to know what's going on.
---
bod
On 5/4/2023 7:21 PM, Bryan O'Donoghue wrote:
> On 04/05/2023 14:49, Bryan O'Donoghue wrote:
>> On 04/05/2023 11:36, Dikshita Agarwal wrote:
>>> This series includes the changes to
>>> - add V4L2_PIX_FMT_P010 as supported decoder format.
>>> - consider dpb color format while calculating buffer
>>> size for dpb buffers.
>>> - update dpb and opb color format when bit depth
>>> changes is detected, also update preferred color
>>> format to P010 in this case.
>>>
>>> With this series, divided the previous version [1] into
>>> multiple patches as suggested in review comments.
>>>
>>> [1]
>>> https://patchwork.linuxtv.org/project/linux-media/list/?series=10376
>>>
>>> Dikshita Agarwal (4):
>>> venus: add support for V4L2_PIX_FMT_P010 color format
>>> venus: update calculation for dpb buffers
>>> venus: add handling of bit depth change from firmwar
>>> venus: return P010 as preferred format for 10 bit decode
>>>
>>> drivers/media/platform/qcom/venus/helpers.c | 24
>>> ++++++++++++++++++++++
>>> drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++
>>> .../media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++-
>>> drivers/media/platform/qcom/venus/vdec.c | 16
>>> +++++++++++++--
>>> 4 files changed, 48 insertions(+), 3 deletions(-)
>>>
>>
>> For future reference a series like this should:
>>
>> 1. Include a log of what changed between the last series and this
>> 2. Describe which comments you addressed
>> I generally try to say
>> "Added newline to dts - Konrad"
>> "Sent the series as a -v3 - Bryan"
>> etc
>> 3. Ideally provide a link to the previous series in
>>
>> https://lore.kernel.org/linux-arm-msm/[email protected]/
>>
>> 4. Use versioning
>> This set should be prefixed with "v2-0000-cover-letter"
>> "v2-0001-add-support" etc
>>
>> "git format-patch mybase..targettip --cover-letter -v2"
>>
>> ---
>> bod
>
> Doh I see you did most of that - just missed the V2.
>
> Please remember to version your subsequent series. "git format-patch -v2"
Does this qualify for a version upgrade when a single patch is
subsequently raised as series ? IMO, the link
to previous single patch in cover letter and then starting the series
(as v0) seems to provide the required info.
>
> ---
> bod
On 4.05.2023 12:36, Dikshita Agarwal wrote:
> If bit depth is detected as 10 bit by firmware, return
> P010 as preferred decoder format to the client.
>
> Signed-off-by: Dikshita Agarwal <[email protected]>
> ---
> drivers/media/platform/qcom/venus/vdec.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 69f7f6e..ed11dc2 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -1468,8 +1468,13 @@ static void vdec_event_change(struct venus_inst *inst,
> inst->out_width = ev_data->width;
> inst->out_height = ev_data->height;
>
> - if (inst->bit_depth != ev_data->bit_depth)
> + if (inst->bit_depth != ev_data->bit_depth) {
> inst->bit_depth = ev_data->bit_depth;
> + if (inst->bit_depth == VIDC_BITDEPTH_10)
> + inst->fmt_cap = &vdec_formats[3];
> + else
> + inst->fmt_cap = &vdec_formats[0];
This doesn't scale and is very error-prone, please enumerate the
entries and assign it using the enumerator, like:
enum {
VDEC_FORMAT_FOO,
...
};
... vdec_formats[] = {
[VDEC_FORMAT_FOO] = { foo, bar, baz }
}
Konrad
> + }
>
> if (inst->pic_struct != ev_data->pic_struct)
> inst->pic_struct = ev_data->pic_struct;
On 4.05.2023 12:36, Dikshita Agarwal wrote:
> add V4L2_PIX_FMT_P010 as supported color format for decoder.
>
> Signed-off-by: Dikshita Agarwal <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
> drivers/media/platform/qcom/venus/helpers.c | 2 ++
> drivers/media/platform/qcom/venus/vdec.c | 4 ++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index ab6a29f..5946def 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -612,6 +612,8 @@ static u32 to_hfi_raw_fmt(u32 v4l2_fmt)
> return HFI_COLOR_FORMAT_NV12_UBWC;
> case V4L2_PIX_FMT_QC10C:
> return HFI_COLOR_FORMAT_YUV420_TP10_UBWC;
> + case V4L2_PIX_FMT_P010:
> + return HFI_COLOR_FORMAT_P010;
> default:
> break;
> }
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 4ceaba3..687d62e 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -43,6 +43,10 @@ static const struct venus_format vdec_formats[] = {
> .num_planes = 1,
> .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> }, {
> + .pixfmt = V4L2_PIX_FMT_P010,
> + .num_planes = 1,
> + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> + }, {
> .pixfmt = V4L2_PIX_FMT_MPEG4,
> .num_planes = 1,
> .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
On 4.05.2023 19:29, Konrad Dybcio wrote:
>
>
> On 4.05.2023 12:36, Dikshita Agarwal wrote:
>> Use dpb color format, width and height of output port
>> for calculating buffer size of dpb buffers.
>>
>> Signed-off-by: Dikshita Agarwal <[email protected]>
>> ---
> Looks sane but I'm not exactly an expert on this
>
> Acked-by: Konrad Dybcio <[email protected]>
>
> Konrad
>> drivers/media/platform/qcom/venus/helpers.c | 4 ++++
>> drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++
>> drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++-
>> 3 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
>> index 5946def..4ad6232 100644
>> --- a/drivers/media/platform/qcom/venus/helpers.c
>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>> @@ -641,12 +641,16 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype,
>> if (is_dec) {
>> params.width = inst->width;
>> params.height = inst->height;
>> + params.out_width = inst->out_width;
>> + params.out_height = inst->out_height;
>> params.codec = inst->fmt_out->pixfmt;
>> params.hfi_color_fmt = to_hfi_raw_fmt(inst->fmt_cap->pixfmt);
>> params.dec.max_mbs_per_frame = mbs_per_frame_max(inst);
>> params.dec.buffer_size_limit = 0;
>> params.dec.is_secondary_output =
>> inst->opb_buftype == HFI_BUFFER_OUTPUT2;
>> + if (params.dec.is_secondary_output)
>> + params.hfi_dpb_color_fmt = inst->dpb_fmt;
>> params.dec.is_interlaced =
>> inst->pic_struct != HFI_INTERLACE_FRAME_PROGRESSIVE;
>> } else {
>> diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
>> index 52a51a3..25e6074 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
>> +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
>> @@ -12,8 +12,11 @@
>> struct hfi_plat_buffers_params {
>> u32 width;
>> u32 height;
>> + u32 out_width;
>> + u32 out_height;
>> u32 codec;
>> u32 hfi_color_fmt;
>> + u32 hfi_dpb_color_fmt;
>> enum hfi_version version;
>> u32 num_vpp_pipes;
>> union {
>> diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
>> index ea25c45..3855b04 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
>> @@ -1185,6 +1185,7 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype,
>> enum hfi_version version = params->version;
>> u32 codec = params->codec;
>> u32 width = params->width, height = params->height, out_min_count;
>> + u32 out_width = params->out_width, out_height = params->out_height;
>> struct dec_bufsize_ops *dec_ops;
>> bool is_secondary_output = params->dec.is_secondary_output;
>> bool is_interlaced = params->dec.is_interlaced;
>> @@ -1235,7 +1236,12 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype,
>> bufreq->count_min = out_min_count;
>> bufreq->size =
>> venus_helper_get_framesz_raw(params->hfi_color_fmt,
>> - width, height);
>> + out_width, out_height);
>> + if (buftype == HFI_BUFFER_OUTPUT &&
>> + params->dec.is_secondary_output)
I suppose this line could be unbroken
Konrad
>> + bufreq->size =
>> + venus_helper_get_framesz_raw(params->hfi_dpb_color_fmt,
>> + out_width, out_height);
>> } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH(version)) {
>> bufreq->size = dec_ops->scratch(width, height, is_interlaced);
>> } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH_1(version)) {
On 4.05.2023 12:36, Dikshita Agarwal wrote:
> Set opb format to TP10_UWC and dpb to client set format
> when bit depth change to 10 bit is detecting by firmware.
>
> Signed-off-by: Dikshita Agarwal <[email protected]>
> ---
> drivers/media/platform/qcom/venus/helpers.c | 18 ++++++++++++++++++
> drivers/media/platform/qcom/venus/vdec.c | 5 ++++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index 4ad6232..4f48ebd 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -1770,6 +1770,24 @@ int venus_helper_get_out_fmts(struct venus_inst *inst, u32 v4l2_fmt,
> if (!caps)
> return -EINVAL;
>
> + if (inst->bit_depth == VIDC_BITDEPTH_10 &&
> + inst->session_type == VIDC_SESSION_TYPE_DEC) {
This could be unbroken
> + found_ubwc = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT,
> + HFI_COLOR_FORMAT_YUV420_TP10_UBWC);
> + found = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT2,
> + fmt);
This could be unbroken
Otherwise I think this lgtm
Acked-by: Konrad Dybcio <[email protected]>
Konrad
> + if (found_ubwc && found) {
> + /*
> + * Hard-code DPB buffers to be 10bit UBWC
> + * until V4L2 is able to expose compressed/tiled
> + * formats to applications.
> + */
> + *out_fmt = HFI_COLOR_FORMAT_YUV420_TP10_UBWC;
> + *out2_fmt = fmt;
> + return 0;
> + }
> + }
> +
> if (ubwc) {
> ubwc_fmt = fmt | HFI_COLOR_FORMAT_UBWC_BASE;
> found_ubwc = find_fmt_from_caps(caps, HFI_BUFFER_OUTPUT,
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 687d62e..69f7f6e 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -701,6 +701,9 @@ static int vdec_set_work_route(struct venus_inst *inst)
> }
>
> #define is_ubwc_fmt(fmt) (!!((fmt) & HFI_COLOR_FORMAT_UBWC_BASE))
> +#define is_10bit_ubwc_fmt(fmt) (!!((fmt) & HFI_COLOR_FORMAT_10_BIT_BASE & \
> + HFI_COLOR_FORMAT_UBWC_BASE))
> +
>
> static int vdec_output_conf(struct venus_inst *inst)
> {
> @@ -748,7 +751,7 @@ static int vdec_output_conf(struct venus_inst *inst)
> inst->opb_fmt = out2_fmt;
> inst->dpb_buftype = HFI_BUFFER_OUTPUT;
> inst->dpb_fmt = out_fmt;
> - } else if (is_ubwc_fmt(out2_fmt)) {
> + } else if (is_ubwc_fmt(out2_fmt) || is_10bit_ubwc_fmt(out_fmt)) {
> inst->opb_buftype = HFI_BUFFER_OUTPUT;
> inst->opb_fmt = out_fmt;
> inst->dpb_buftype = HFI_BUFFER_OUTPUT2;
On 4.05.2023 12:36, Dikshita Agarwal wrote:
> Use dpb color format, width and height of output port
> for calculating buffer size of dpb buffers.
>
> Signed-off-by: Dikshita Agarwal <[email protected]>
> ---
Looks sane but I'm not exactly an expert on this
Acked-by: Konrad Dybcio <[email protected]>
Konrad
> drivers/media/platform/qcom/venus/helpers.c | 4 ++++
> drivers/media/platform/qcom/venus/hfi_plat_bufs.h | 3 +++
> drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c | 8 +++++++-
> 3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index 5946def..4ad6232 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -641,12 +641,16 @@ static int platform_get_bufreq(struct venus_inst *inst, u32 buftype,
> if (is_dec) {
> params.width = inst->width;
> params.height = inst->height;
> + params.out_width = inst->out_width;
> + params.out_height = inst->out_height;
> params.codec = inst->fmt_out->pixfmt;
> params.hfi_color_fmt = to_hfi_raw_fmt(inst->fmt_cap->pixfmt);
> params.dec.max_mbs_per_frame = mbs_per_frame_max(inst);
> params.dec.buffer_size_limit = 0;
> params.dec.is_secondary_output =
> inst->opb_buftype == HFI_BUFFER_OUTPUT2;
> + if (params.dec.is_secondary_output)
> + params.hfi_dpb_color_fmt = inst->dpb_fmt;
> params.dec.is_interlaced =
> inst->pic_struct != HFI_INTERLACE_FRAME_PROGRESSIVE;
> } else {
> diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
> index 52a51a3..25e6074 100644
> --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
> +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs.h
> @@ -12,8 +12,11 @@
> struct hfi_plat_buffers_params {
> u32 width;
> u32 height;
> + u32 out_width;
> + u32 out_height;
> u32 codec;
> u32 hfi_color_fmt;
> + u32 hfi_dpb_color_fmt;
> enum hfi_version version;
> u32 num_vpp_pipes;
> union {
> diff --git a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
> index ea25c45..3855b04 100644
> --- a/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
> +++ b/drivers/media/platform/qcom/venus/hfi_plat_bufs_v6.c
> @@ -1185,6 +1185,7 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype,
> enum hfi_version version = params->version;
> u32 codec = params->codec;
> u32 width = params->width, height = params->height, out_min_count;
> + u32 out_width = params->out_width, out_height = params->out_height;
> struct dec_bufsize_ops *dec_ops;
> bool is_secondary_output = params->dec.is_secondary_output;
> bool is_interlaced = params->dec.is_interlaced;
> @@ -1235,7 +1236,12 @@ static int bufreq_dec(struct hfi_plat_buffers_params *params, u32 buftype,
> bufreq->count_min = out_min_count;
> bufreq->size =
> venus_helper_get_framesz_raw(params->hfi_color_fmt,
> - width, height);
> + out_width, out_height);
> + if (buftype == HFI_BUFFER_OUTPUT &&
> + params->dec.is_secondary_output)
> + bufreq->size =
> + venus_helper_get_framesz_raw(params->hfi_dpb_color_fmt,
> + out_width, out_height);
> } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH(version)) {
> bufreq->size = dec_ops->scratch(width, height, is_interlaced);
> } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH_1(version)) {
On 5/4/2023 10:50 PM, Konrad Dybcio wrote:
>
> On 4.05.2023 12:36, Dikshita Agarwal wrote:
>> If bit depth is detected as 10 bit by firmware, return
>> P010 as preferred decoder format to the client.
>>
>> Signed-off-by: Dikshita Agarwal <[email protected]>
>> ---
>> drivers/media/platform/qcom/venus/vdec.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index 69f7f6e..ed11dc2 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -1468,8 +1468,13 @@ static void vdec_event_change(struct venus_inst *inst,
>> inst->out_width = ev_data->width;
>> inst->out_height = ev_data->height;
>>
>> - if (inst->bit_depth != ev_data->bit_depth)
>> + if (inst->bit_depth != ev_data->bit_depth) {
>> inst->bit_depth = ev_data->bit_depth;
>> + if (inst->bit_depth == VIDC_BITDEPTH_10)
>> + inst->fmt_cap = &vdec_formats[3];
>> + else
>> + inst->fmt_cap = &vdec_formats[0];
> This doesn't scale and is very error-prone, please enumerate the
> entries and assign it using the enumerator, like:
>
> enum {
> VDEC_FORMAT_FOO,
> ...
> };
>
> ... vdec_formats[] = {
> [VDEC_FORMAT_FOO] = { foo, bar, baz }
> }
>
> Konrad
I agree, this can be improved but I would prefer making that change as
separate patch.
As this is not only related to HDR 10 decoding, there are other places
in the code which will require similar change.
Thanks,
Dikshita
>> + }
>>
>> if (inst->pic_struct != ev_data->pic_struct)
>> inst->pic_struct = ev_data->pic_struct;
On 5.05.2023 11:03, Dikshita Agarwal wrote:
>
> On 5/4/2023 10:50 PM, Konrad Dybcio wrote:
>>
>> On 4.05.2023 12:36, Dikshita Agarwal wrote:
>>> If bit depth is detected as 10 bit by firmware, return
>>> P010 as preferred decoder format to the client.
>>>
>>> Signed-off-by: Dikshita Agarwal <[email protected]>
>>> ---
>>> drivers/media/platform/qcom/venus/vdec.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>>> index 69f7f6e..ed11dc2 100644
>>> --- a/drivers/media/platform/qcom/venus/vdec.c
>>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>>> @@ -1468,8 +1468,13 @@ static void vdec_event_change(struct venus_inst *inst,
>>> inst->out_width = ev_data->width;
>>> inst->out_height = ev_data->height;
>>> - if (inst->bit_depth != ev_data->bit_depth)
>>> + if (inst->bit_depth != ev_data->bit_depth) {
>>> inst->bit_depth = ev_data->bit_depth;
>>> + if (inst->bit_depth == VIDC_BITDEPTH_10)
>>> + inst->fmt_cap = &vdec_formats[3];
>>> + else
>>> + inst->fmt_cap = &vdec_formats[0];
>> This doesn't scale and is very error-prone, please enumerate the
>> entries and assign it using the enumerator, like:
>>
>> enum {
>> VDEC_FORMAT_FOO,
>> ...
>> };
>>
>> ... vdec_formats[] = {
>> [VDEC_FORMAT_FOO] = { foo, bar, baz }
>> }
>>
>> Konrad
>
> I agree, this can be improved but I would prefer making that change as separate patch.
>
> As this is not only related to HDR 10 decoding, there are other places in the code which will require similar change.
This is not a very strong argument.
You can't add code that will break very easily whenever somebody touches
that array and pinky-promise to fix it some time later, just because you
want to get your feature merged faster, this is not drivers/staging.
Konrad
>
> Thanks,
>
> Dikshita
>
>>> + }
>>> if (inst->pic_struct != ev_data->pic_struct)
>>> inst->pic_struct = ev_data->pic_struct;
On 05/05/2023 12:03, Dikshita Agarwal wrote:
>
> On 5/4/2023 10:50 PM, Konrad Dybcio wrote:
>>
>> On 4.05.2023 12:36, Dikshita Agarwal wrote:
>>> If bit depth is detected as 10 bit by firmware, return
>>> P010 as preferred decoder format to the client.
>>>
>>> Signed-off-by: Dikshita Agarwal <[email protected]>
>>> ---
>>> drivers/media/platform/qcom/venus/vdec.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/vdec.c
>>> b/drivers/media/platform/qcom/venus/vdec.c
>>> index 69f7f6e..ed11dc2 100644
>>> --- a/drivers/media/platform/qcom/venus/vdec.c
>>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>>> @@ -1468,8 +1468,13 @@ static void vdec_event_change(struct
>>> venus_inst *inst,
>>> inst->out_width = ev_data->width;
>>> inst->out_height = ev_data->height;
>>> - if (inst->bit_depth != ev_data->bit_depth)
>>> + if (inst->bit_depth != ev_data->bit_depth) {
>>> inst->bit_depth = ev_data->bit_depth;
>>> + if (inst->bit_depth == VIDC_BITDEPTH_10)
>>> + inst->fmt_cap = &vdec_formats[3];
>>> + else
>>> + inst->fmt_cap = &vdec_formats[0];
>> This doesn't scale and is very error-prone, please enumerate the
>> entries and assign it using the enumerator, like:
>>
>> enum {
>> VDEC_FORMAT_FOO,
>> ...
>> };
>>
>> ... vdec_formats[] = {
>> [VDEC_FORMAT_FOO] = { foo, bar, baz }
>> }
>>
>> Konrad
>
> I agree, this can be improved but I would prefer making that change as
> separate patch.
Good!
>
> As this is not only related to HDR 10 decoding, there are other places
> in the code which will require similar change.
Please fix them first. Adding more cruft is not a good way to go.
>
> Thanks,
>
> Dikshita
>
>>> + }
>>> if (inst->pic_struct != ev_data->pic_struct)
>>> inst->pic_struct = ev_data->pic_struct;
--
With best wishes
Dmitry