| Message ID | 1702899149-21321-1-git-send-email-quic_dikshita@quicinc.com |
|---|---|
| Headers | show |
| Series | Qualcomm video encoder and decoder driver | expand |
Hi Dmitry, On 12/19/2023 12:08 AM, Dmitry Baryshkov wrote: > On 18/12/2023 13:31, Dikshita Agarwal wrote: >> This patch series introduces support for Qualcomm new video acceleration >> hardware architecture, used for video stream decoding/encoding. This driver >> is based on new communication protocol between video hardware and application >> processor. > > This doesn't answer one important point, you have been asked for v1. What is the > actual change point between Venus and Iris? What has been changed so much that > it demands a separate driver. This is the main question for the cover letter, > which has not been answered so far. > > From what I see from you bindings, the hardware is pretty close to what we see > in the latest venus generations. I asssme that there was a change in the vcodec > inteface to the firmware and other similar changes. Could you please point out, > which parts of Venus driver do no longer work or are not applicable for sm8550 The motivation behind having a separate IRIS driver was discussed earlier in [1] In the same discussion, it was ellaborated on how the impact would be with change in the new firmware interface and other video layers in the driver. I can add this in cover letter in the next revision. We see some duplication of code and to handle the same, the series brings in a common code reusability between iris and venus. Aligning the common peices of venus and iris will be a work in progress, once we land the base driver for iris. Again qualcomm video team does not have a plan to support sm8550/x1e80100 on venus as the changes are too interleaved to absorb in venus driver. And there is significant interest in community to start validating video driver on sm8550 or x1e80100. [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ Regards, Vikash
On 12/19/2023 12:02 AM, Dmitry Baryshkov wrote: > On 18/12/2023 13:31, Dikshita Agarwal wrote: >> Introduce helper to calculate size of pixel buffer and >> use in venus driver. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >> --- >> drivers/media/platform/qcom/vcodec/buffers.c | 103 ++++++++++++ >> drivers/media/platform/qcom/vcodec/buffers.h | 15 ++ >> drivers/media/platform/qcom/vcodec/venus/Makefile | 4 +- >> drivers/media/platform/qcom/vcodec/venus/helpers.c | 172 >> ++++----------------- >> drivers/media/platform/qcom/vcodec/venus/helpers.h | 2 +- >> .../platform/qcom/vcodec/venus/hfi_plat_bufs.h | 4 +- >> .../platform/qcom/vcodec/venus/hfi_plat_bufs_v6.c | 10 +- >> drivers/media/platform/qcom/vcodec/venus/vdec.c | 5 +- >> 8 files changed, 157 insertions(+), 158 deletions(-) >> create mode 100644 drivers/media/platform/qcom/vcodec/buffers.c >> create mode 100644 drivers/media/platform/qcom/vcodec/buffers.h >> >> diff --git a/drivers/media/platform/qcom/vcodec/buffers.c >> b/drivers/media/platform/qcom/vcodec/buffers.c >> new file mode 100644 >> index 0000000..58ab3b0 >> --- /dev/null >> +++ b/drivers/media/platform/qcom/vcodec/buffers.c >> @@ -0,0 +1,103 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights >> reserved. >> + */ >> +#include <linux/videodev2.h> >> + >> +#include "buffers.h" >> + >> +u32 video_raw_buffer_size(u32 colorformat, > > Usual drill, non-prefixed name. Will fix in next version. > >> + u32 pix_width, >> + u32 pix_height) > > My gut tells me that this is misaligned. > >> +{ >> + u32 size = 0; >> + u32 y_plane, uv_plane, y_stride, >> + uv_stride, y_sclines, uv_sclines; >> + u32 y_ubwc_plane = 0, uv_ubwc_plane = 0; >> + u32 y_meta_stride = 0, y_meta_scanlines = 0; >> + u32 uv_meta_stride = 0, uv_meta_scanlines = 0; >> + u32 y_meta_plane = 0, uv_meta_plane = 0; >> + >> + if (!pix_width || !pix_height) >> + goto invalid_input; >> + >> + switch (colorformat) { >> + case V4L2_PIX_FMT_NV12: >> + case V4L2_PIX_FMT_NV21: >> + y_stride = ALIGN(pix_width, 128); >> + uv_stride = ALIGN(pix_width, 128); >> + y_sclines = ALIGN(pix_height, 32); >> + uv_sclines = ALIGN((pix_height + 1) >> 1, 16); >> + y_plane = y_stride * y_sclines; >> + uv_plane = uv_stride * uv_sclines; >> + size = y_plane + uv_plane; >> + break; >> + case V4L2_PIX_FMT_QC08C: >> + y_stride = ALIGN(pix_width, 128); >> + uv_stride = ALIGN(pix_width, 128); >> + y_sclines = ALIGN(pix_height, 32); >> + uv_sclines = ALIGN((pix_height + 1) >> 1, 32); >> + y_meta_stride = ALIGN(DIV_ROUND_UP(pix_width, 32), 64); >> + uv_meta_stride = ALIGN(DIV_ROUND_UP((pix_width + 1) >> 1, 16), 64); >> + y_ubwc_plane = >> + ALIGN(y_stride * y_sclines, 4096); >> + uv_ubwc_plane = >> + ALIGN(uv_stride * uv_sclines, 4096); >> + y_meta_scanlines = >> + ALIGN(DIV_ROUND_UP(pix_height, 8), 16); >> + y_meta_plane = >> + ALIGN(y_meta_stride * y_meta_scanlines, 4096); >> + uv_meta_scanlines = >> + ALIGN(DIV_ROUND_UP((pix_height + 1) >> 1, 8), 16); >> + uv_meta_plane = >> + ALIGN(uv_meta_stride * uv_meta_scanlines, 4096); >> + size = (y_ubwc_plane + uv_ubwc_plane + y_meta_plane + >> + uv_meta_plane); >> + break; >> + case V4L2_PIX_FMT_QC10C: >> + y_stride = >> + ALIGN(ALIGN(pix_width, 192) * 4 / 3, 256); >> + uv_stride = >> + ALIGN(ALIGN(pix_width, 192) * 4 / 3, 256); >> + y_sclines = >> + ALIGN(pix_height, 16); >> + uv_sclines = >> + ALIGN((pix_height + 1) >> 1, 16); >> + y_ubwc_plane = >> + ALIGN(y_stride * y_sclines, 4096); >> + uv_ubwc_plane = >> + ALIGN(uv_stride * uv_sclines, 4096); >> + y_meta_stride = >> + ALIGN(DIV_ROUND_UP(pix_width, 48), 64); >> + y_meta_scanlines = >> + ALIGN(DIV_ROUND_UP(pix_height, 4), 16); >> + y_meta_plane = >> + ALIGN(y_meta_stride * y_meta_scanlines, 4096); >> + uv_meta_stride = >> + ALIGN(DIV_ROUND_UP((pix_width + 1) >> 1, 24), 64); >> + uv_meta_scanlines = >> + ALIGN(DIV_ROUND_UP((pix_height + 1) >> 1, 4), 16); >> + uv_meta_plane = >> + ALIGN(uv_meta_stride * uv_meta_scanlines, 4096); >> + >> + size = y_ubwc_plane + uv_ubwc_plane + y_meta_plane + >> + uv_meta_plane; >> + break; >> + case V4L2_PIX_FMT_P010: >> + y_stride = ALIGN(pix_width * 2, 128); >> + uv_stride = ALIGN(pix_width * 2, 128); >> + y_sclines = ALIGN(pix_height, 32); >> + uv_sclines = ALIGN((pix_height + 1) >> 1, 16); >> + y_plane = y_stride * y_sclines; >> + uv_plane = uv_stride * uv_sclines; >> + >> + size = y_plane + uv_plane; >> + break; >> + default: >> + break; >> + } >> + >> +invalid_input: >> + >> + return ALIGN(size, 4096); >> +} >> diff --git a/drivers/media/platform/qcom/vcodec/buffers.h >> b/drivers/media/platform/qcom/vcodec/buffers.h >> new file mode 100644 >> index 0000000..ac1d052 >> --- /dev/null >> +++ b/drivers/media/platform/qcom/vcodec/buffers.h >> @@ -0,0 +1,15 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights >> reserved. >> + */ >> + >> +#ifndef _BUFFERS_H_ >> +#define _BUFFERS_H_ >> + >> +#include <linux/types.h> >> + >> +u32 video_raw_buffer_size(u32 colorformat, >> + u32 pix_width, >> + u32 pix_height); >> + >> +#endif >> diff --git a/drivers/media/platform/qcom/vcodec/venus/Makefile >> b/drivers/media/platform/qcom/vcodec/venus/Makefile >> index 1941ef4..6abd54a 100644 >> --- a/drivers/media/platform/qcom/vcodec/venus/Makefile >> +++ b/drivers/media/platform/qcom/vcodec/venus/Makefile >> @@ -1,8 +1,7 @@ >> # SPDX-License-Identifier: GPL-2.0 >> # Makefile for Qualcomm Venus driver >> -venus-core-objs += ../firmware.o \ >> - ../hfi_queue.o >> +venus-core-objs += ../firmware.o ../hfi_queue.o ../buffers.o > > Ugh. I missed that in the previous patches. This is not how the helpers are > done. Add normal kernel module instead. > I unnderstand this looks little messy, I can try to move the makefile one level up (in new vcodec folder), compile the common object by default and compile venus or iris based on config. Please let me know what do you think of this apporach. >> venus-core-objs += core.o helpers.o firmware_no_tz.o \ >> hfi_venus.o hfi_msgs.o hfi_cmds.o hfi.o \ >> @@ -10,6 +9,7 @@ venus-core-objs += core.o helpers.o firmware_no_tz.o \ >> hfi_platform.o hfi_platform_v4.o \ >> hfi_platform_v6.o hfi_plat_bufs_v6.o \ >> +venus-dec-objs += ../buffers.o > > I really wonder, why doesn't this end up with 'symbol defined multiple > times' error. > >> venus-dec-objs += vdec.o vdec_ctrls.o >> venus-enc-objs += venc.o venc_ctrls.o >> diff --git a/drivers/media/platform/qcom/vcodec/venus/helpers.c >> b/drivers/media/platform/qcom/vcodec/venus/helpers.c >> index 8295542..95e4424 100644 >> --- a/drivers/media/platform/qcom/vcodec/venus/helpers.c >> +++ b/drivers/media/platform/qcom/vcodec/venus/helpers.c >> @@ -12,6 +12,7 @@ >> #include <media/v4l2-mem2mem.h> >> #include <asm/div64.h> >> +#include "../buffers.h" >> #include "core.h" >> #include "helpers.h" >> #include "hfi_helper.h" >> @@ -616,6 +617,27 @@ static u32 to_hfi_raw_fmt(u32 v4l2_fmt) >> return 0; >> } >> +u32 to_v4l2_raw_fmt(u32 hfi_color_fmt) >> +{ >> + switch (hfi_color_fmt) { >> + case HFI_COLOR_FORMAT_NV12: >> + return V4L2_PIX_FMT_NV12; >> + case HFI_COLOR_FORMAT_NV21: >> + return V4L2_PIX_FMT_NV21; >> + case HFI_COLOR_FORMAT_NV12_UBWC: >> + return V4L2_PIX_FMT_QC08C; >> + case HFI_COLOR_FORMAT_YUV420_TP10_UBWC: >> + return V4L2_PIX_FMT_QC10C; >> + case HFI_COLOR_FORMAT_P010: >> + return V4L2_PIX_FMT_P010; >> + default: >> + break; >> + } >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(to_v4l2_raw_fmt); >> + >> static int platform_get_bufreq(struct venus_inst *inst, u32 buftype, >> struct hfi_buffer_requirements *req) >> { >> @@ -639,20 +661,20 @@ static int platform_get_bufreq(struct venus_inst >> *inst, u32 buftype, >> 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.color_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.dpb_color_fmt = to_v4l2_raw_fmt(inst->dpb_fmt); >> params.dec.is_interlaced = >> inst->pic_struct != HFI_INTERLACE_FRAME_PROGRESSIVE; >> } else { >> params.width = inst->out_width; >> params.height = inst->out_height; >> params.codec = inst->fmt_cap->pixfmt; >> - params.hfi_color_fmt = to_hfi_raw_fmt(inst->fmt_out->pixfmt); >> + params.color_fmt = inst->fmt_out->pixfmt; >> params.enc.work_mode = VIDC_WORK_MODE_2; >> params.enc.rc_type = HFI_RATE_CONTROL_OFF; >> if (enc_ctr->bitrate_mode == V4L2_MPEG_VIDEO_BITRATE_MODE_CQ) >> @@ -942,146 +964,10 @@ int venus_helper_set_profile_level(struct >> venus_inst *inst, u32 profile, u32 lev >> } >> EXPORT_SYMBOL_GPL(venus_helper_set_profile_level); >> -static u32 get_framesize_raw_nv12(u32 width, u32 height) >> -{ >> - u32 y_stride, uv_stride, y_plane; >> - u32 y_sclines, uv_sclines, uv_plane; >> - u32 size; >> - >> - y_stride = ALIGN(width, 128); >> - uv_stride = ALIGN(width, 128); >> - y_sclines = ALIGN(height, 32); >> - uv_sclines = ALIGN(((height + 1) >> 1), 16); >> - >> - y_plane = y_stride * y_sclines; >> - uv_plane = uv_stride * uv_sclines + SZ_4K; >> - size = y_plane + uv_plane + SZ_8K; >> - >> - return ALIGN(size, SZ_4K); >> -} >> - >> -static u32 get_framesize_raw_nv12_ubwc(u32 width, u32 height) >> -{ >> - u32 y_meta_stride, y_meta_plane; >> - u32 y_stride, y_plane; >> - u32 uv_meta_stride, uv_meta_plane; >> - u32 uv_stride, uv_plane; >> - u32 extradata = SZ_16K; >> - >> - y_meta_stride = ALIGN(DIV_ROUND_UP(width, 32), 64); >> - y_meta_plane = y_meta_stride * ALIGN(DIV_ROUND_UP(height, 8), 16); >> - y_meta_plane = ALIGN(y_meta_plane, SZ_4K); >> - >> - y_stride = ALIGN(width, 128); >> - y_plane = ALIGN(y_stride * ALIGN(height, 32), SZ_4K); >> - >> - uv_meta_stride = ALIGN(DIV_ROUND_UP(width / 2, 16), 64); >> - uv_meta_plane = uv_meta_stride * ALIGN(DIV_ROUND_UP(height / 2, 8), >> 16); >> - uv_meta_plane = ALIGN(uv_meta_plane, SZ_4K); >> - >> - uv_stride = ALIGN(width, 128); >> - uv_plane = ALIGN(uv_stride * ALIGN(height / 2, 32), SZ_4K); >> - >> - return ALIGN(y_meta_plane + y_plane + uv_meta_plane + uv_plane + >> - max(extradata, y_stride * 48), SZ_4K); >> -} >> - >> -static u32 get_framesize_raw_p010(u32 width, u32 height) >> -{ >> - u32 y_plane, uv_plane, y_stride, uv_stride, y_sclines, uv_sclines; >> - >> - y_stride = ALIGN(width * 2, 128); >> - uv_stride = ALIGN(width * 2, 128); >> - y_sclines = ALIGN(height, 32); >> - uv_sclines = ALIGN((height + 1) >> 1, 16); >> - y_plane = y_stride * y_sclines; >> - uv_plane = uv_stride * uv_sclines; >> - >> - return ALIGN((y_plane + uv_plane), SZ_4K); >> -} >> - >> -static u32 get_framesize_raw_p010_ubwc(u32 width, u32 height) >> -{ >> - u32 y_stride, uv_stride, y_sclines, uv_sclines; >> - u32 y_ubwc_plane, uv_ubwc_plane; >> - u32 y_meta_stride, y_meta_scanlines; >> - u32 uv_meta_stride, uv_meta_scanlines; >> - u32 y_meta_plane, uv_meta_plane; >> - u32 size; >> - >> - y_stride = ALIGN(width * 2, 256); >> - uv_stride = ALIGN(width * 2, 256); >> - y_sclines = ALIGN(height, 16); >> - uv_sclines = ALIGN((height + 1) >> 1, 16); >> - >> - y_ubwc_plane = ALIGN(y_stride * y_sclines, SZ_4K); >> - uv_ubwc_plane = ALIGN(uv_stride * uv_sclines, SZ_4K); >> - y_meta_stride = ALIGN(DIV_ROUND_UP(width, 32), 64); >> - y_meta_scanlines = ALIGN(DIV_ROUND_UP(height, 4), 16); >> - y_meta_plane = ALIGN(y_meta_stride * y_meta_scanlines, SZ_4K); >> - uv_meta_stride = ALIGN(DIV_ROUND_UP((width + 1) >> 1, 16), 64); >> - uv_meta_scanlines = ALIGN(DIV_ROUND_UP((height + 1) >> 1, 4), 16); >> - uv_meta_plane = ALIGN(uv_meta_stride * uv_meta_scanlines, SZ_4K); >> - >> - size = y_ubwc_plane + uv_ubwc_plane + y_meta_plane + uv_meta_plane; >> - >> - return ALIGN(size, SZ_4K); >> -} >> - >> -static u32 get_framesize_raw_yuv420_tp10_ubwc(u32 width, u32 height) >> -{ >> - u32 y_stride, uv_stride, y_sclines, uv_sclines; >> - u32 y_ubwc_plane, uv_ubwc_plane; >> - u32 y_meta_stride, y_meta_scanlines; >> - u32 uv_meta_stride, uv_meta_scanlines; >> - u32 y_meta_plane, uv_meta_plane; >> - u32 extradata = SZ_16K; >> - u32 size; >> - >> - y_stride = ALIGN(width * 4 / 3, 256); >> - uv_stride = ALIGN(width * 4 / 3, 256); >> - y_sclines = ALIGN(height, 16); >> - uv_sclines = ALIGN((height + 1) >> 1, 16); >> - >> - y_ubwc_plane = ALIGN(y_stride * y_sclines, SZ_4K); >> - uv_ubwc_plane = ALIGN(uv_stride * uv_sclines, SZ_4K); >> - y_meta_stride = ALIGN(DIV_ROUND_UP(width, 48), 64); >> - y_meta_scanlines = ALIGN(DIV_ROUND_UP(height, 4), 16); >> - y_meta_plane = ALIGN(y_meta_stride * y_meta_scanlines, SZ_4K); >> - uv_meta_stride = ALIGN(DIV_ROUND_UP((width + 1) >> 1, 24), 64); >> - uv_meta_scanlines = ALIGN(DIV_ROUND_UP((height + 1) >> 1, 4), 16); >> - uv_meta_plane = ALIGN(uv_meta_stride * uv_meta_scanlines, SZ_4K); >> - >> - size = y_ubwc_plane + uv_ubwc_plane + y_meta_plane + uv_meta_plane; >> - size += max(extradata + SZ_8K, y_stride * 48); >> - >> - return ALIGN(size, SZ_4K); >> -} >> - >> -u32 venus_helper_get_framesz_raw(u32 hfi_fmt, u32 width, u32 height) >> -{ >> - switch (hfi_fmt) { >> - case HFI_COLOR_FORMAT_NV12: >> - case HFI_COLOR_FORMAT_NV21: >> - return get_framesize_raw_nv12(width, height); >> - case HFI_COLOR_FORMAT_NV12_UBWC: >> - return get_framesize_raw_nv12_ubwc(width, height); >> - case HFI_COLOR_FORMAT_P010: >> - return get_framesize_raw_p010(width, height); >> - case HFI_COLOR_FORMAT_P010_UBWC: >> - return get_framesize_raw_p010_ubwc(width, height); >> - case HFI_COLOR_FORMAT_YUV420_TP10_UBWC: >> - return get_framesize_raw_yuv420_tp10_ubwc(width, height); >> - default: >> - return 0; >> - } >> -} >> -EXPORT_SYMBOL_GPL(venus_helper_get_framesz_raw); >> - >> u32 venus_helper_get_framesz(u32 v4l2_fmt, u32 width, u32 height) >> { >> - u32 hfi_fmt, sz; >> bool compressed; >> + u32 sz; >> switch (v4l2_fmt) { >> case V4L2_PIX_FMT_MPEG: >> @@ -1112,11 +998,7 @@ u32 venus_helper_get_framesz(u32 v4l2_fmt, u32 >> width, u32 height) >> return ALIGN(sz, SZ_4K); >> } >> - hfi_fmt = to_hfi_raw_fmt(v4l2_fmt); >> - if (!hfi_fmt) >> - return 0; >> - >> - return venus_helper_get_framesz_raw(hfi_fmt, width, height); >> + return video_raw_buffer_size(v4l2_fmt, width, height); >> } >> EXPORT_SYMBOL_GPL(venus_helper_get_framesz); >> diff --git a/drivers/media/platform/qcom/vcodec/venus/helpers.h >> b/drivers/media/platform/qcom/vcodec/venus/helpers.h >> index 358e4f3..9b72d18 100644 >> --- a/drivers/media/platform/qcom/vcodec/venus/helpers.h >> +++ b/drivers/media/platform/qcom/vcodec/venus/helpers.h >> @@ -11,6 +11,7 @@ >> struct venus_inst; >> struct venus_core; >> +u32 to_v4l2_raw_fmt(u32 hfi_color_fmt); >> bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt); >> struct vb2_v4l2_buffer *venus_helper_find_buf(struct venus_inst *inst, >> unsigned int type, u32 idx); >> @@ -29,7 +30,6 @@ void venus_helper_m2m_device_run(void *priv); >> void venus_helper_m2m_job_abort(void *priv); >> int venus_helper_get_bufreq(struct venus_inst *inst, u32 type, >> struct hfi_buffer_requirements *req); >> -u32 venus_helper_get_framesz_raw(u32 hfi_fmt, u32 width, u32 height); >> u32 venus_helper_get_framesz(u32 v4l2_fmt, u32 width, u32 height); >> int venus_helper_set_input_resolution(struct venus_inst *inst, >> unsigned int width, unsigned int height); >> diff --git a/drivers/media/platform/qcom/vcodec/venus/hfi_plat_bufs.h >> b/drivers/media/platform/qcom/vcodec/venus/hfi_plat_bufs.h >> index 25e6074..20f684e 100644 >> --- a/drivers/media/platform/qcom/vcodec/venus/hfi_plat_bufs.h >> +++ b/drivers/media/platform/qcom/vcodec/venus/hfi_plat_bufs.h >> @@ -15,8 +15,8 @@ struct hfi_plat_buffers_params { >> u32 out_width; >> u32 out_height; >> u32 codec; >> - u32 hfi_color_fmt; >> - u32 hfi_dpb_color_fmt; >> + u32 color_fmt; >> + u32 dpb_color_fmt; > > As usual. This is not a helper introduction. This is field rename. Could > you please split that. > >> enum hfi_version version; >> u32 num_vpp_pipes; >> union { >> diff --git a/drivers/media/platform/qcom/vcodec/venus/hfi_plat_bufs_v6.c >> b/drivers/media/platform/qcom/vcodec/venus/hfi_plat_bufs_v6.c >> index f5a6559..3e06516 100644 >> --- a/drivers/media/platform/qcom/vcodec/venus/hfi_plat_bufs_v6.c >> +++ b/drivers/media/platform/qcom/vcodec/venus/hfi_plat_bufs_v6.c >> @@ -6,6 +6,7 @@ >> #include <linux/sizes.h> >> #include <linux/videodev2.h> >> +#include "../buffers.h" >> #include "hfi.h" >> #include "hfi_plat_bufs.h" >> #include "helpers.h" >> @@ -1233,13 +1234,11 @@ static int bufreq_dec(struct >> hfi_plat_buffers_params *params, u32 buftype, >> buffer_size_limit); >> } else if (buftype == HFI_BUFFER_OUTPUT || buftype == >> HFI_BUFFER_OUTPUT2) { >> hfi_bufreq_set_count_min(bufreq, version, out_min_count); >> - bufreq->size = >> - venus_helper_get_framesz_raw(params->hfi_color_fmt, >> + bufreq->size = video_raw_buffer_size(params->color_fmt, >> 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, >> + bufreq->size = video_raw_buffer_size(params->dpb_color_fmt, >> out_width, out_height); >> } else if (buftype == HFI_BUFFER_INTERNAL_SCRATCH(version)) { >> bufreq->size = dec_ops->scratch(width, height, is_interlaced); >> @@ -1297,8 +1296,7 @@ static int bufreq_enc(struct >> hfi_plat_buffers_params *params, u32 buftype, >> if (buftype == HFI_BUFFER_INPUT) { >> hfi_bufreq_set_count_min(bufreq, version, MIN_INPUT_BUFFERS); >> - bufreq->size = >> - venus_helper_get_framesz_raw(params->hfi_color_fmt, >> + bufreq->size = video_raw_buffer_size(params->color_fmt, >> width, height); >> } else if (buftype == HFI_BUFFER_OUTPUT || >> buftype == HFI_BUFFER_OUTPUT2) { >> diff --git a/drivers/media/platform/qcom/vcodec/venus/vdec.c >> b/drivers/media/platform/qcom/vcodec/venus/vdec.c >> index dbf305c..e6316be 100644 >> --- a/drivers/media/platform/qcom/vcodec/venus/vdec.c >> +++ b/drivers/media/platform/qcom/vcodec/venus/vdec.c >> @@ -15,6 +15,7 @@ >> #include <media/v4l2-mem2mem.h> >> #include <media/videobuf2-dma-contig.h> >> +#include "../buffers.h" >> #include "hfi_venus_io.h" >> #include "hfi_parser.h" >> #include "core.h" >> @@ -777,9 +778,9 @@ static int vdec_output_conf(struct venus_inst *inst) >> return ret; >> inst->output_buf_size = >> - venus_helper_get_framesz_raw(out_fmt, width, height); >> + video_raw_buffer_size(to_v4l2_raw_fmt(out_fmt), width, height); >> inst->output2_buf_size = >> - venus_helper_get_framesz_raw(out2_fmt, width, height); >> + video_raw_buffer_size(to_v4l2_raw_fmt(out2_fmt), width, >> height); >> if (is_ubwc_fmt(out_fmt)) { >> inst->opb_buftype = HFI_BUFFER_OUTPUT2; >
On 12/19/2023 12:16 AM, Dmitry Baryshkov wrote: > On 18/12/2023 13:32, Dikshita Agarwal wrote: >> Introduces different states for core. State transitions >> are defined, based on which they would be allowed/rejected/ignored. >> >> IRIS_CORE_DEINIT - default state. >> IRIS_CORE_INIT_WAIT - wait state for video core to complete >> initialization. >> IRIS_CORE_INIT - core state with core initialized. FW loaded and >> HW brought out of reset, shared queues established between host >> driver and firmware. >> IRIS_CORE_ERROR - error state. >> >> | >> v >> ----------- >> +--->| DEINIT |<---+ >> | ----------- | >> | | | >> | v | >> | ----------- | >> | |INIT_WAIT| | >> | ----------- | >> | / \ | >> | / \ | >> | / \ | >> | v v v >> ----------- ----------. >> | INIT |-->| ERROR | >> ----------- ----------. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > > Can we see users for this API please? Otherwise it's just a dead code. > This is like a preparation patch to add the core state. These APIs are used in patch-10[1] onward. [1]: https://patchwork.kernel.org/project/linux-media/patch/1702899149-21321-11-git-send-email-quic_dikshita@quicinc.com/ >> --- >> drivers/media/platform/qcom/vcodec/iris/Makefile | 3 +- >> .../media/platform/qcom/vcodec/iris/iris_core.h | 4 ++ >> .../media/platform/qcom/vcodec/iris/iris_state.c | 64 >> ++++++++++++++++++++++ >> .../media/platform/qcom/vcodec/iris/iris_state.h | 22 ++++++++ >> 4 files changed, 92 insertions(+), 1 deletion(-) >> create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_state.c >> create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_state.h >> >> diff --git a/drivers/media/platform/qcom/vcodec/iris/Makefile >> b/drivers/media/platform/qcom/vcodec/iris/Makefile >> index 0748819..12a74de 100644 >> --- a/drivers/media/platform/qcom/vcodec/iris/Makefile >> +++ b/drivers/media/platform/qcom/vcodec/iris/Makefile >> @@ -1,4 +1,5 @@ >> iris-objs += iris_probe.o \ >> - resources.o >> + resources.o \ >> + iris_state.o >> obj-$(CONFIG_VIDEO_QCOM_IRIS) += iris.o >> diff --git a/drivers/media/platform/qcom/vcodec/iris/iris_core.h >> b/drivers/media/platform/qcom/vcodec/iris/iris_core.h >> index c2bc95d..56a5b7d 100644 >> --- a/drivers/media/platform/qcom/vcodec/iris/iris_core.h >> +++ b/drivers/media/platform/qcom/vcodec/iris/iris_core.h >> @@ -9,6 +9,8 @@ >> #include <linux/types.h> >> #include <media/v4l2-device.h> >> +#include "iris_state.h" >> + >> /** >> * struct iris_core - holds core parameters valid for all instances >> * >> @@ -27,6 +29,7 @@ >> * @clk_count: count of iris clocks >> * @reset_tbl: table of iris reset clocks >> * @reset_count: count of iris reset clocks >> + * @state: current state of core >> */ >> struct iris_core { >> @@ -45,6 +48,7 @@ struct iris_core { >> u32 clk_count; >> struct reset_info *reset_tbl; >> u32 reset_count; >> + enum iris_core_state state; >> }; >> #endif >> diff --git a/drivers/media/platform/qcom/vcodec/iris/iris_state.c >> b/drivers/media/platform/qcom/vcodec/iris/iris_state.c >> new file mode 100644 >> index 0000000..22557af >> --- /dev/null >> +++ b/drivers/media/platform/qcom/vcodec/iris/iris_state.c >> @@ -0,0 +1,64 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights >> reserved. >> + */ >> + >> +#include "iris_core.h" >> +#include "iris_state.h" >> + >> +#define IRIS_STATE(name)[IRIS_CORE_##name] = "CORE_"#name > > Inline this please. > >> + >> +static const char * const core_state_names[] = { >> + IRIS_STATE(DEINIT), >> + IRIS_STATE(INIT_WAIT), >> + IRIS_STATE(INIT), >> + IRIS_STATE(ERROR), >> +}; >> + >> +#undef IRIS_STATE >> + >> +bool core_in_valid_state(struct iris_core *core) > > So, even in your driver you have global names? That's really ugh. Please > fix them. > Sure will fix these names. >> +{ >> + return core->state == IRIS_CORE_INIT || >> + core->state == IRIS_CORE_INIT_WAIT; >> +} >> + >> +static const char *core_state_name(enum iris_core_state state) >> +{ >> + if ((unsigned int)state < ARRAY_SIZE(core_state_names)) >> + return core_state_names[state]; >> + >> + return "UNKNOWN_STATE"; >> +} >> + >> +static bool iris_allow_core_state_change(struct iris_core *core, >> + enum iris_core_state req_state) >> +{ >> + if (core->state == IRIS_CORE_DEINIT) >> + return req_state == IRIS_CORE_INIT_WAIT || req_state == >> IRIS_CORE_ERROR; >> + else if (core->state == IRIS_CORE_INIT_WAIT) >> + return req_state == IRIS_CORE_INIT || req_state == IRIS_CORE_ERROR; >> + else if (core->state == IRIS_CORE_INIT) >> + return req_state == IRIS_CORE_DEINIT || req_state == >> IRIS_CORE_ERROR; >> + else if (core->state == IRIS_CORE_ERROR) >> + return req_state == IRIS_CORE_DEINIT; >> + >> + dev_warn(core->dev, "core state change %s -> %s is not allowed\n", >> + core_state_name(core->state), core_state_name(req_state)); >> + >> + return false; >> +} >> + >> +int iris_change_core_state(struct iris_core *core, >> + enum iris_core_state request_state) >> +{ >> + if (core->state == request_state) >> + return 0; >> + >> + if (!iris_allow_core_state_change(core, request_state)) >> + return -EINVAL; >> + >> + core->state = request_state; >> + >> + return 0; >> +} >> diff --git a/drivers/media/platform/qcom/vcodec/iris/iris_state.h >> b/drivers/media/platform/qcom/vcodec/iris/iris_state.h >> new file mode 100644 >> index 0000000..ee20842 >> --- /dev/null >> +++ b/drivers/media/platform/qcom/vcodec/iris/iris_state.h >> @@ -0,0 +1,22 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights >> reserved. >> + */ >> + >> +#ifndef _IRIS_STATE_H_ >> +#define _IRIS_STATE_H_ >> + >> +struct iris_core; >> + >> +enum iris_core_state { >> + IRIS_CORE_DEINIT, >> + IRIS_CORE_INIT_WAIT, >> + IRIS_CORE_INIT, >> + IRIS_CORE_ERROR, >> +}; >> + >> +bool core_in_valid_state(struct iris_core *core); >> +int iris_change_core_state(struct iris_core *core, >> + enum iris_core_state request_state); >> + >> +#endif >
On 12/19/2023 3:03 AM, Konrad Dybcio wrote: > > > On 12/18/23 12:32, Dikshita Agarwal wrote: >> Shared queues are used for communication between driver and firmware. >> There are 3 types of queues: >> Command queue - driver to write any command to firmware. >> Message queue - firmware to send any response to driver. >> Debug queue - firmware to write debug message. >> >> Above queues are initialized and configured to firmware during probe. >> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >> --- > [...] > >> + ret = hfi_queue_init(core->dev, &core->iface_q_table, &core->sfr, >> + &core->command_queue, &core->message_queue, >> + &core->debug_queue, core); >> + if (ret) { >> + dev_err_probe(core->dev, ret, >> + "%s: interface queues init failed\n", __func__); >> + goto err_vdev_unreg; >> + } >> + >> return ret; > Like before, you're suppose to return dev_err_probe Sure, will fix all such instances. > > Konrad
Hi, On 12/20/2023 1:07 PM, Dmitry Baryshkov wrote: > On Wed, 20 Dec 2023 at 08:32, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: >> >> Hi Dmitry, >> >> On 12/19/2023 12:08 AM, Dmitry Baryshkov wrote: >>> On 18/12/2023 13:31, Dikshita Agarwal wrote: >>>> This patch series introduces support for Qualcomm new video acceleration >>>> hardware architecture, used for video stream decoding/encoding. This driver >>>> is based on new communication protocol between video hardware and application >>>> processor. >>> >>> This doesn't answer one important point, you have been asked for v1. What is the >>> actual change point between Venus and Iris? What has been changed so much that >>> it demands a separate driver. This is the main question for the cover letter, >>> which has not been answered so far. >>> >>> From what I see from you bindings, the hardware is pretty close to what we see >>> in the latest venus generations. I asssme that there was a change in the vcodec >>> inteface to the firmware and other similar changes. Could you please point out, >>> which parts of Venus driver do no longer work or are not applicable for sm8550 >> >> The motivation behind having a separate IRIS driver was discussed earlier in [1] >> In the same discussion, it was ellaborated on how the impact would be with >> change in the new firmware interface and other video layers in the driver. I can >> add this in cover letter in the next revision. > > Ok. So the changes cover the HFI interface. Is that correct? Change wise, yes. >> We see some duplication of code and to handle the same, the series brings in a >> common code reusability between iris and venus. Aligning the common peices of >> venus and iris will be a work in progress, once we land the base driver for iris. > > This is not how it usually works. Especially not with the patches you > have posted. > > I have the following suggestion how this story can continue: > You can _start_ by reworking venus driver, separating the HFI / > firmware / etc interface to an internal interface in the driver. Then > implement Iris as a plug in for that interface. I might be mistaken > here, but I think this is the way how this can be beneficial for both > the video en/decoding on both old and new platforms. HFI/firmware interface is already confined to HFI layer in the existing venus driver. We explained in the previous discussion [1], on how the HFI change impacts the other layers by taking example of a DRC usecase. Please have a look considering the usecase and the impact it brings to other layers in the driver. [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ > Short rationale: > The venus driver has a history of supported platforms. There is > already some kind of buffer management in place. Both developers and > testers have spent their effort on finding issues there. Sending new > driver means that we have to spend the same amount of efforts on this. > Moreover, even from the porter point of view. You are creating new > bindings for the new hardware. Which do not follow the > venus-common.yaml. And they do not follow the defined bindings for the > recent venus platforms. Which means that as a developer I have to care > about two different ways to describe nearly the same hardware.>> Again qualcomm video team does not have a plan to support sm8550/x1e80100 on >> venus as the changes are too interleaved to absorb in venus driver. And there is >> significant interest in community to start validating video driver on sm8550 or >> x1e80100. >> >> [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ >> >> Regards, >> Vikash > > >
On 20/12/2023 07:32, Vikash Garodia wrote: >> From what I see from you bindings, the hardware is pretty close to what we see >> in the latest venus generations. I asssme that there was a change in the vcodec >> inteface to the firmware and other similar changes. Could you please point out, >> which parts of Venus driver do no longer work or are not applicable for sm8550 > > The motivation behind having a separate IRIS driver was discussed earlier in [1] > In the same discussion, it was ellaborated on how the impact would be with > change in the new firmware interface and other video layers in the driver. I can > add this in cover letter in the next revision. > > We see some duplication of code and to handle the same, the series brings in a > common code reusability between iris and venus. Aligning the common peices of > venus and iris will be a work in progress, once we land the base driver for iris. > > Again qualcomm video team does not have a plan to support sm8550/x1e80100 on If you want it to get merged, then create such plan, please. > venus as the changes are too interleaved to absorb in venus driver. And there is > significant interest in community to start validating video driver on sm8550 or > x1e80100. Community does not want duplicated drivers leading to maintenance costs. Your approach to this is not how upstreaming process works. Best regards, Krzysztof
On 12/20/2023 2:09 PM, Dmitry Baryshkov wrote: > On Wed, 20 Dec 2023 at 10:14, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: >> >> Hi, >> >> On 12/20/2023 1:07 PM, Dmitry Baryshkov wrote: >>> On Wed, 20 Dec 2023 at 08:32, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: >>>> >>>> Hi Dmitry, >>>> >>>> On 12/19/2023 12:08 AM, Dmitry Baryshkov wrote: >>>>> On 18/12/2023 13:31, Dikshita Agarwal wrote: >>>>>> This patch series introduces support for Qualcomm new video acceleration >>>>>> hardware architecture, used for video stream decoding/encoding. This driver >>>>>> is based on new communication protocol between video hardware and application >>>>>> processor. >>>>> >>>>> This doesn't answer one important point, you have been asked for v1. What is the >>>>> actual change point between Venus and Iris? What has been changed so much that >>>>> it demands a separate driver. This is the main question for the cover letter, >>>>> which has not been answered so far. >>>>> >>>>> From what I see from you bindings, the hardware is pretty close to what we see >>>>> in the latest venus generations. I asssme that there was a change in the vcodec >>>>> inteface to the firmware and other similar changes. Could you please point out, >>>>> which parts of Venus driver do no longer work or are not applicable for sm8550 >>>> >>>> The motivation behind having a separate IRIS driver was discussed earlier in [1] >>>> In the same discussion, it was ellaborated on how the impact would be with >>>> change in the new firmware interface and other video layers in the driver. I can >>>> add this in cover letter in the next revision. >>> >>> Ok. So the changes cover the HFI interface. Is that correct? >> Change wise, yes. >> >>>> We see some duplication of code and to handle the same, the series brings in a >>>> common code reusability between iris and venus. Aligning the common peices of >>>> venus and iris will be a work in progress, once we land the base driver for iris. >>> >>> This is not how it usually works. Especially not with the patches you >>> have posted. >>> >>> I have the following suggestion how this story can continue: >>> You can _start_ by reworking venus driver, separating the HFI / >>> firmware / etc interface to an internal interface in the driver. Then >>> implement Iris as a plug in for that interface. I might be mistaken >>> here, but I think this is the way how this can be beneficial for both >>> the video en/decoding on both old and new platforms. >> >> HFI/firmware interface is already confined to HFI layer in the existing venus >> driver. We explained in the previous discussion [1], on how the HFI change >> impacts the other layers by taking example of a DRC usecase. Please have a look >> considering the usecase and the impact it brings to other layers in the driver. > > I have looked at it. And I still see huge change in the interface > side, but it doesn't tell me about the hardware changes. I hope you noticed how the common layers like decoder, response, state layers are impacted in handling one of usecase. Now add that to all the different scenarios like seek, drain, DRC during seek, DRC during drain, etc. > Have you evaluated the other opportunity? > > To have a common platform interface and firmware-specific backend? > > You have already done a part of it, but from a different perspective. > You have tried to move common code out of the driver. Instead we are > asking you to do a different thing. Move non-common code within the > driver. Then add your code on top of that. For common platform - yes, we are bringing in common stuff like PIL. Other than that, abstraction to firmware interface is not that confined to one layer. It spreads over decoder/encoder/common layers. Now when majority of the layers/code is different, we planned to make it in parallel to venus and have a common layer having common things to both iris and venus. >> >> [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ >>> Short rationale: >>> The venus driver has a history of supported platforms. There is >>> already some kind of buffer management in place. Both developers and >>> testers have spent their effort on finding issues there. Sending new >>> driver means that we have to spend the same amount of efforts on this. >>> Moreover, even from the porter point of view. You are creating new >>> bindings for the new hardware. Which do not follow the >>> venus-common.yaml. And they do not follow the defined bindings for the >>> recent venus platforms. Which means that as a developer I have to care >>> about two different ways to describe nearly the same hardware.>> Again qualcomm video team does not have a plan to support sm8550/x1e80100 on >>>> venus as the changes are too interleaved to absorb in venus driver. And there is >>>> significant interest in community to start validating video driver on sm8550 or >>>> x1e80100. >>>> >>>> [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ >>>> >>>> Regards, >>>> Vikash >>> >>> >>> > > >
Hi Dmitry and Krzysztof On 12/20/2023 1:52 AM, Dmitry Baryshkov wrote: > On Wed, 20 Dec 2023 at 10:53, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: >> >> On 12/20/2023 2:09 PM, Dmitry Baryshkov wrote: >>> On Wed, 20 Dec 2023 at 10:14, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: >>>> >>>> Hi, >>>> >>>> On 12/20/2023 1:07 PM, Dmitry Baryshkov wrote: >>>>> On Wed, 20 Dec 2023 at 08:32, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: >>>>>> >>>>>> Hi Dmitry, >>>>>> >>>>>> On 12/19/2023 12:08 AM, Dmitry Baryshkov wrote: >>>>>>> On 18/12/2023 13:31, Dikshita Agarwal wrote: >>>>>>>> This patch series introduces support for Qualcomm new video acceleration >>>>>>>> hardware architecture, used for video stream decoding/encoding. This driver >>>>>>>> is based on new communication protocol between video hardware and application >>>>>>>> processor. >>>>>>> >>>>>>> This doesn't answer one important point, you have been asked for v1. What is the >>>>>>> actual change point between Venus and Iris? What has been changed so much that >>>>>>> it demands a separate driver. This is the main question for the cover letter, >>>>>>> which has not been answered so far. >>>>>>> >>>>>>> From what I see from you bindings, the hardware is pretty close to what we see >>>>>>> in the latest venus generations. I asssme that there was a change in the vcodec >>>>>>> inteface to the firmware and other similar changes. Could you please point out, >>>>>>> which parts of Venus driver do no longer work or are not applicable for sm8550 >>>>>> >>>>>> The motivation behind having a separate IRIS driver was discussed earlier in [1] >>>>>> In the same discussion, it was ellaborated on how the impact would be with >>>>>> change in the new firmware interface and other video layers in the driver. I can >>>>>> add this in cover letter in the next revision. >>>>> >>>>> Ok. So the changes cover the HFI interface. Is that correct? >>>> Change wise, yes. >>>> >>>>>> We see some duplication of code and to handle the same, the series brings in a >>>>>> common code reusability between iris and venus. Aligning the common peices of >>>>>> venus and iris will be a work in progress, once we land the base driver for iris. >>>>> >>>>> This is not how it usually works. Especially not with the patches you >>>>> have posted. >>>>> >>>>> I have the following suggestion how this story can continue: >>>>> You can _start_ by reworking venus driver, separating the HFI / >>>>> firmware / etc interface to an internal interface in the driver. Then >>>>> implement Iris as a plug in for that interface. I might be mistaken >>>>> here, but I think this is the way how this can be beneficial for both >>>>> the video en/decoding on both old and new platforms. >>>> >>>> HFI/firmware interface is already confined to HFI layer in the existing venus >>>> driver. We explained in the previous discussion [1], on how the HFI change >>>> impacts the other layers by taking example of a DRC usecase. Please have a look >>>> considering the usecase and the impact it brings to other layers in the driver. >>> >>> I have looked at it. And I still see huge change in the interface >>> side, but it doesn't tell me about the hardware changes. >> >> I hope you noticed how the common layers like decoder, response, state layers >> are impacted in handling one of usecase. Now add that to all the different >> scenarios like seek, drain, DRC during seek, DRC during drain, etc. > > Yes, for sure. > >> >>> Have you evaluated the other opportunity? >>> >>> To have a common platform interface and firmware-specific backend? >>> >>> You have already done a part of it, but from a different perspective. >>> You have tried to move common code out of the driver. Instead we are >>> asking you to do a different thing. Move non-common code within the >>> driver. Then add your code on top of that. >> >> For common platform - yes, we are bringing in common stuff like PIL. >> Other than that, abstraction to firmware interface is not that confined to one >> layer. It spreads over decoder/encoder/common layers. Now when majority of the >> layers/code is different, we planned to make it in parallel to venus and have a >> common layer having common things to both iris and venus. > > My suggestion still holds. Start with this common platform code. > Rather than arguing back and forth, could you please perform an > experiment on the current venus driver and move firmware interface to > subdirs, leaving just the platform / PIL / formats / etc in place? > This will at least allow us to determine whether it is a feasible > concept or not. > Your comments are valid. The platform driver piece and some other pieces still are common between venus and iris despite this initial effort of moving common pieces out. I have also seen this from whatever I have reviewed. Video team also acknowledges this part and internally I think they did evaluate this option and their feedback was, the more and more they changed, they were touching pretty much every file of venus. The missing piece i think in all this discussion is that in addition to the forward channel, the reverse channel of HFI, based on which the rest of the video state machine changes should also be considered. So even with respect to the code layout, its not just the forward communication but the backwards communication of fw--->hfi--->codec is becoming a challenge as the venus layers seem to only work with the hfi of venus. For adding support for the new HFI events/communication, it was getting harder to extend venus. What I certainly acknowledge is now with iris whether this can be dealt with better for future chipsets to avoid another re-write for another HFI as that will be too much of maintenance cost. I will let the video team comment on this part. Thanks Abhinav >> >>>> >>>> [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ >>>>> Short rationale: >>>>> The venus driver has a history of supported platforms. There is >>>>> already some kind of buffer management in place. Both developers and >>>>> testers have spent their effort on finding issues there. Sending new >>>>> driver means that we have to spend the same amount of efforts on this. >>>>> Moreover, even from the porter point of view. You are creating new >>>>> bindings for the new hardware. Which do not follow the >>>>> venus-common.yaml. And they do not follow the defined bindings for the >>>>> recent venus platforms. Which means that as a developer I have to care >>>>> about two different ways to describe nearly the same hardware.>> Again qualcomm video team does not have a plan to support sm8550/x1e80100 on >>>>>> venus as the changes are too interleaved to absorb in venus driver. And there is >>>>>> significant interest in community to start validating video driver on sm8550 or >>>>>> x1e80100. >>>>>> >>>>>> [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ >>>>>> >>>>>> Regards, >>>>>> Vikash >>>>> >>>>> >>>>> >>> >>> >>> > > >
Hi Abhinav, On Wed, 20 Dec 2023 at 20:55, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > Hi Dmitry and Krzysztof > > On 12/20/2023 1:52 AM, Dmitry Baryshkov wrote: > > On Wed, 20 Dec 2023 at 10:53, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: > >> > >> On 12/20/2023 2:09 PM, Dmitry Baryshkov wrote: > >>> On Wed, 20 Dec 2023 at 10:14, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: > >>>> > >>>> Hi, > >>>> > >>>> On 12/20/2023 1:07 PM, Dmitry Baryshkov wrote: > >>>>> On Wed, 20 Dec 2023 at 08:32, Vikash Garodia <quic_vgarodia@quicinc.com> wrote: > >>>>>> > >>>>>> Hi Dmitry, > >>>>>> > >>>>>> On 12/19/2023 12:08 AM, Dmitry Baryshkov wrote: > >>>>>>> On 18/12/2023 13:31, Dikshita Agarwal wrote: > >>>>>>>> This patch series introduces support for Qualcomm new video acceleration > >>>>>>>> hardware architecture, used for video stream decoding/encoding. This driver > >>>>>>>> is based on new communication protocol between video hardware and application > >>>>>>>> processor. > >>>>>>> > >>>>>>> This doesn't answer one important point, you have been asked for v1. What is the > >>>>>>> actual change point between Venus and Iris? What has been changed so much that > >>>>>>> it demands a separate driver. This is the main question for the cover letter, > >>>>>>> which has not been answered so far. > >>>>>>> > >>>>>>> From what I see from you bindings, the hardware is pretty close to what we see > >>>>>>> in the latest venus generations. I asssme that there was a change in the vcodec > >>>>>>> inteface to the firmware and other similar changes. Could you please point out, > >>>>>>> which parts of Venus driver do no longer work or are not applicable for sm8550 > >>>>>> > >>>>>> The motivation behind having a separate IRIS driver was discussed earlier in [1] > >>>>>> In the same discussion, it was ellaborated on how the impact would be with > >>>>>> change in the new firmware interface and other video layers in the driver. I can > >>>>>> add this in cover letter in the next revision. > >>>>> > >>>>> Ok. So the changes cover the HFI interface. Is that correct? > >>>> Change wise, yes. > >>>> > >>>>>> We see some duplication of code and to handle the same, the series brings in a > >>>>>> common code reusability between iris and venus. Aligning the common peices of > >>>>>> venus and iris will be a work in progress, once we land the base driver for iris. > >>>>> > >>>>> This is not how it usually works. Especially not with the patches you > >>>>> have posted. > >>>>> > >>>>> I have the following suggestion how this story can continue: > >>>>> You can _start_ by reworking venus driver, separating the HFI / > >>>>> firmware / etc interface to an internal interface in the driver. Then > >>>>> implement Iris as a plug in for that interface. I might be mistaken > >>>>> here, but I think this is the way how this can be beneficial for both > >>>>> the video en/decoding on both old and new platforms. > >>>> > >>>> HFI/firmware interface is already confined to HFI layer in the existing venus > >>>> driver. We explained in the previous discussion [1], on how the HFI change > >>>> impacts the other layers by taking example of a DRC usecase. Please have a look > >>>> considering the usecase and the impact it brings to other layers in the driver. > >>> > >>> I have looked at it. And I still see huge change in the interface > >>> side, but it doesn't tell me about the hardware changes. > >> > >> I hope you noticed how the common layers like decoder, response, state layers > >> are impacted in handling one of usecase. Now add that to all the different > >> scenarios like seek, drain, DRC during seek, DRC during drain, etc. > > > > Yes, for sure. > > > >> > >>> Have you evaluated the other opportunity? > >>> > >>> To have a common platform interface and firmware-specific backend? > >>> > >>> You have already done a part of it, but from a different perspective. > >>> You have tried to move common code out of the driver. Instead we are > >>> asking you to do a different thing. Move non-common code within the > >>> driver. Then add your code on top of that. > >> > >> For common platform - yes, we are bringing in common stuff like PIL. > >> Other than that, abstraction to firmware interface is not that confined to one > >> layer. It spreads over decoder/encoder/common layers. Now when majority of the > >> layers/code is different, we planned to make it in parallel to venus and have a > >> common layer having common things to both iris and venus. > > > > My suggestion still holds. Start with this common platform code. > > Rather than arguing back and forth, could you please perform an > > experiment on the current venus driver and move firmware interface to > > subdirs, leaving just the platform / PIL / formats / etc in place? > > This will at least allow us to determine whether it is a feasible > > concept or not. > > > > Your comments are valid. The platform driver piece and some other pieces > still are common between venus and iris despite this initial effort of > moving common pieces out. I have also seen this from whatever I have > reviewed. > > Video team also acknowledges this part and internally I think they did > evaluate this option and their feedback was, the more and more they > changed, they were touching pretty much every file of venus. This is fine from my POV. Splitting the the common functionality also touched a significant part of the venus driver in a pretty bad way. > The missing piece i think in all this discussion is that in addition to > the forward channel, the reverse channel of HFI, based on which the rest > of the video state machine changes should also be considered. > > So even with respect to the code layout, its not just the forward > communication but the backwards communication of fw--->hfi--->codec is > becoming a challenge as the venus layers seem to only work with the hfi > of venus. Again, this is a question of the platform backend part. Nobody questions that newer platforms have their own driver interface. We are not asking your team to add if(new) { foo; } else { bar; } conditions all over the code. Instead we have been asking to split all platform specifics to the 'backend'. Compare this with mdp4, mdp5 and dpu backends of the single drm/msm frontend. Or with a similar approach found in other DRM drivers. Adding new driver is frequently unjustified, instead the existing driver gets extended to support different generations. > For adding support for the new HFI events/communication, it was getting > harder to extend venus. > > What I certainly acknowledge is now with iris whether this can be dealt > with better for future chipsets to avoid another re-write for another > HFI as that will be too much of maintenance cost. Another approach, which might also be fine, if that looks better from your point of view. The current venus driver might have issues with the internal interfaces. Or it might be not suitable for the backends. In this case, can we please get older platforms also supported by the iris driver? This will be a very simple case then: the old deprecated driver, being phased out and removed, and a new one, which is being phased in. The problem is very simple from my side. I do not want to end up in a situation where we have to change platform code for both drivers if there is any common issue. Neither would I like for venus and iris bindings to diverge more than necessary. And as you have seen from my comments, the iris bindings already do not follow the venus example. > I will let the video team comment on this part. > > Thanks > > Abhinav > > > > > > >> > >>>> > >>>> [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ > >>>>> Short rationale: > >>>>> The venus driver has a history of supported platforms. There is > >>>>> already some kind of buffer management in place. Both developers and > >>>>> testers have spent their effort on finding issues there. Sending new > >>>>> driver means that we have to spend the same amount of efforts on this. > >>>>> Moreover, even from the porter point of view. You are creating new > >>>>> bindings for the new hardware. Which do not follow the > >>>>> venus-common.yaml. And they do not follow the defined bindings for the > >>>>> recent venus platforms. Which means that as a developer I have to care > >>>>> about two different ways to describe nearly the same hardware.>> Again qualcomm video team does not have a plan to support sm8550/x1e80100 on > >>>>>> venus as the changes are too interleaved to absorb in venus driver. And there is > >>>>>> significant interest in community to start validating video driver on sm8550 or > >>>>>> x1e80100. > >>>>>> > >>>>>> [1] https://lore.kernel.org/lkml/8c97d866-1cab-0106-4ab3-3ca070945ef7@quicinc.com/ > >>>>>> > >>>>>> Regards, > >>>>>> Vikash
On 20.12.2023 09:04, Dikshita Agarwal wrote: > > > On 12/18/2023 8:39 PM, Konrad Dybcio wrote: >> On 18.12.2023 12:32, Dikshita Agarwal wrote: >>> Add support for initializing Iris "resources", which are clocks, >>> interconnects, power domains, reset clocks, and clock frequencies >>> used for Iris hardware. >>> >>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >>> --- [...] >>> + >>> + for (i = 0; i < core->clk_count; i++) { >>> + cinfo = &core->clock_tbl[i]; >>> + cinfo->name = plat_clk_table[i].name; >>> + cinfo->clk_id = plat_clk_table[i].clk_id; >>> + cinfo->has_scaling = plat_clk_table[i].has_scaling; >>> + cinfo->clk = devm_clk_get(core->dev, cinfo->name); >>> + if (IS_ERR(cinfo->clk)) { >>> + dev_err(core->dev, >>> + "%s: failed to get clock: %s\n", __func__, cinfo->name); >>> + return PTR_ERR(cinfo->clk); >>> + } >>> + } >> Are you not going to use OPP for scaling the main RPMhPD with the core >> clock? >> > We are using OPP for scaling the vcodec clk. > Could you please elaborate you query here, may be I didn't understand fully. It's just that this approach of scanning everything we know and expect about the clock seems a bit unnecessary.. Going with the approach I suggested below (i.e. separate struct clk for important ones like core or mem clock) simplify this to the point where you just set opp_set_rate on the imporant ones and you can throw clk_bulk_ operations at the ones that simply need to be en/disabled. Konrad [...] >>> + >>> +struct power_domain_info { >>> + struct device *genpd_dev; >>> + const char *name; >>> +}; >>> + >>> +struct clock_info { >>> + struct clk *clk; >>> + const char *name; >> I'm not sure why you need it >> >>> + u32 clk_id; >> Or this >> >>> + bool has_scaling; >> Or this >> >> you could probably do something like this: >> >> struct big_iris_struct { >> [...] >> struct clk *core_clk; >> struct clk *memory_clk; >> struct clk *some_important_scaled_clock; >> struct clk_bulk_data less_important_nonscaling_clocks[X] >> } >> >> and then make use of all of the great common upstream APIs to manage >> them! >> > Will explore this and get back.
Hello All, On 12/18/2023 5:01 PM, Dikshita Agarwal wrote: > This patch series introduces support for Qualcomm new video acceleration > hardware architecture, used for video stream decoding/encoding. This driver > is based on new communication protocol between video hardware and application > processor. > This driver comes with below capabilities: > - V4L2 complaint video driver with M2M and STREAMING capability. > - Supports H264, H265, VP9 decoders. > - Supports H264, H265 encoders. > This driver comes with below features: > - Centralized resource and memory management. > - Centralized management of core and instance states. > - Defines platform specific capabilities and features. As a results, it provides > a single point of control to enable/disable a given feature depending on > specific platform capabilities. > - Handles hardware interdependent configurations. For a given configuration from > client, the driver checks for hardware dependent configuration/s and updates > the same. > - Handles multiple complex video scenarios involving state transitions - DRC, > Drain, Seek, back to back DRC, DRC during Drain sequence, DRC during Seek > sequence. > - Introduces a flexible way for driver to subscribe for any property with > hardware. Hardware would inform driver with those subscribed property with any > change in value. > - Introduces performance (clock and bus) model based on new hardware > architecture. > - Introduces multi thread safe design to handle communication between client and > hardware. > - Adapts encoder quality improvements available in new hardware architecture. > - Implements asynchronous communication with hardware to achieve better > experience in low latency usecases. > - Supports multi stage hardware architecture for encode/decode. > - Output and capture planes are controlled independently. Thereby providing a > way to reconfigure individual plane. > - Hardware packetization layer supports synchronization between configuration > packet and data packet. > - Introduces a flexibility to receive a hardware response for a given command > packet. > - Native hardware support of LAST flag which is mandatory to align with port > reconfiguration and DRAIN sequence as per V4L guidelines. > - Native hardware support for drain sequence. 1. We considered enhancing the venus driver to support iris functionality but concluded that the result would essentially be two parallel drivers implemented in the same folder. 2. Although the underlying hardware for venus and iris are quite similar, but there are other factors which brings the need of new driver: a. the host firmware interface (HFI) changed between the two. Venus supports HFI protocol 1.0 while iris supports HFI protocol 2.0. b. unlike HFI protocol 1.0, HFI protocol 2.0 is better aligned to V4L2 APIs. c. iris driver modularize platforms, hardware variants, and states to extend it for any upcoming SOC. Thereby more extendable to newer SOCs in future. d. Iris also supports many advanced features. 3. Based on the comments received on posted iris driver [1], we evaluated it further and to better align with the upstream standard and practices, we acknowledge that even though iris driver is the way forward, it would be ideal to bring in the Venus hardwares enabled in this driver. 4. Hence, we decided to rework iris driver to add support of HFI protocol 1.0 as well. 5. Initially we would start with adding support for one HFI protocol 1.0 based platform which would be SM8250. 6. SM8250 enablement on iris driver would be incremental, starting with basic decode for H264 codec. 7. In long-term, all venus supported platforms would be migrated to iris. 8. Iris driver and venus driver will co-exist till remaining supported targets also gets migrated to iris driver. 9. We would continue to support and maintain venus driver during this phased out approach. Please share your thoughts on the above proposal. [1] https://patchwork.kernel.org/project/linux-media/cover/1702899149-21321-1-git-send-email-quic_dikshita@quicinc.com/#25643473 Regards, Vikash
Hi Vikash, On 2/29/24 16:09, Vikash Garodia wrote: > Hello All, > > On 12/18/2023 5:01 PM, Dikshita Agarwal wrote: >> This patch series introduces support for Qualcomm new video acceleration >> hardware architecture, used for video stream decoding/encoding. This driver >> is based on new communication protocol between video hardware and application >> processor. >> This driver comes with below capabilities: >> - V4L2 complaint video driver with M2M and STREAMING capability. >> - Supports H264, H265, VP9 decoders. >> - Supports H264, H265 encoders. >> This driver comes with below features: >> - Centralized resource and memory management. >> - Centralized management of core and instance states. >> - Defines platform specific capabilities and features. As a results, it provides >> a single point of control to enable/disable a given feature depending on >> specific platform capabilities. >> - Handles hardware interdependent configurations. For a given configuration from >> client, the driver checks for hardware dependent configuration/s and updates >> the same. >> - Handles multiple complex video scenarios involving state transitions - DRC, >> Drain, Seek, back to back DRC, DRC during Drain sequence, DRC during Seek >> sequence. >> - Introduces a flexible way for driver to subscribe for any property with >> hardware. Hardware would inform driver with those subscribed property with any >> change in value. >> - Introduces performance (clock and bus) model based on new hardware >> architecture. >> - Introduces multi thread safe design to handle communication between client and >> hardware. >> - Adapts encoder quality improvements available in new hardware architecture. >> - Implements asynchronous communication with hardware to achieve better >> experience in low latency usecases. >> - Supports multi stage hardware architecture for encode/decode. >> - Output and capture planes are controlled independently. Thereby providing a >> way to reconfigure individual plane. >> - Hardware packetization layer supports synchronization between configuration >> packet and data packet. >> - Introduces a flexibility to receive a hardware response for a given command >> packet. >> - Native hardware support of LAST flag which is mandatory to align with port >> reconfiguration and DRAIN sequence as per V4L guidelines. >> - Native hardware support for drain sequence. > > 1. We considered enhancing the venus driver to support iris functionality but > concluded that the result would essentially be two parallel drivers implemented > in the same folder. > 2. Although the underlying hardware for venus and iris are quite similar, but > there are other factors which brings the need of new driver: > a. the host firmware interface (HFI) changed between the two. Venus supports > HFI protocol 1.0 while iris supports HFI protocol 2.0. > b. unlike HFI protocol 1.0, HFI protocol 2.0 is better aligned to V4L2 APIs. > c. iris driver modularize platforms, hardware variants, and states to extend > it for any upcoming SOC. Thereby more extendable to newer SOCs in future. > d. Iris also supports many advanced features. > 3. Based on the comments received on posted iris driver [1], we evaluated it > further and to better align with the upstream standard and practices, we > acknowledge that even though iris driver is the way forward, it would be ideal > to bring in the Venus hardwares enabled in this driver. > 4. Hence, we decided to rework iris driver to add support of HFI protocol 1.0 as > well. > 5. Initially we would start with adding support for one HFI protocol 1.0 based > platform which would be SM8250. > 6. SM8250 enablement on iris driver would be incremental, starting with basic > decode for H264 codec. > 7. In long-term, all venus supported platforms would be migrated to iris. > 8. Iris driver and venus driver will co-exist till remaining supported targets > also gets migrated to iris driver. > 9. We would continue to support and maintain venus driver during this phased out > approach. > > Please share your thoughts on the above proposal. I think this is a reasonable approach: the venus driver is quite old and it was created when we were at more-or-less the same time also developing better codec frameworks. So it is not quite up-to-date with all the latest requirements. Starting with a clean slate for the iris driver and then add support for venus platforms to the iris driver makes sense. Just one serious concern: who will do the venus platform migration? Are there resources available to do that work? Or is this just wishful thinking? Regards, Hans > > [1] > https://patchwork.kernel.org/project/linux-media/cover/1702899149-21321-1-git-send-email-quic_dikshita@quicinc.com/#25643473 > > Regards, > Vikash >
Hi, I'm trying this series of patches for enabling /dev/video0 on sm8650 hdk but failed. After modprobing, lsmod says just like the follwoing: root@hdk8650:/lib/modules/6.7.0-rc3+# lsmod Module Size Used by iris 110592 -2 v4l2_mem2mem 20480 -2 videobuf2_memops 12288 -2 videobuf2_v4l2 20480 -2 videobuf2_common 45056 -2 videodev 176128 -2 This series looks for sm8550 device though, my question is that this series have been tested on the device (sm8650 hdk)? or do you expect this should work on it?
On 12/04/2024 08:13, Hyunjun Ko wrote: > Hi, > > I'm trying this series of patches for enabling /dev/video0 on sm8650 hdk but failed. > After modprobing, lsmod says just like the follwoing: > > root@hdk8650:/lib/modules/6.7.0-rc3+# lsmod > Module Size Used by > iris 110592 -2 > v4l2_mem2mem 20480 -2 > videobuf2_memops 12288 -2 > videobuf2_v4l2 20480 -2 > videobuf2_common 45056 -2 > videodev 176128 -2 > > > This series looks for sm8550 device though, my question is that this series have been tested on the device (sm8650 hdk)? or do you expect this should work on it? > Different SoCs so despite how close they look in version numbers, you'd expect at a minimum some clock and/or power-domain churn - even if there is a 1:1 mapping in register numbers and offsets. We wouldn't ordinarily expect to be able to be able to move SoC versions so easily - 8550 and 8650 have similar SoC version numbers but, this is not necessarily an indicator of silicon IP block version reuse. --- bod
Hi, Thanks for this series of patches. I successfully adjusted these patches and tried to test video features with gstreamer or ffmpeg. But I found this provides staetful interfaces while I need stateless, which might cause an issue for my side.. My question is do you have any plan to implement stateless interfaces or already you have somewhere?
Hi, On 5/16/2024 1:27 PM, Hyunjun Ko wrote: > Hi, > > Thanks for this series of patches. I successfully adjusted these patches and tried to test video features with gstreamer or ffmpeg. > But I found this provides staetful interfaces while I need stateless, which might cause an issue for my side.. > > My question is do you have any plan to implement stateless interfaces or already you have somewhere? There is no plan to implement stateless interfaces. Regards, Vikash
This patch series introduces support for Qualcomm new video acceleration hardware architecture, used for video stream decoding/encoding. This driver is based on new communication protocol between video hardware and application processor. This driver comes with below capabilities: - V4L2 complaint video driver with M2M and STREAMING capability. - Supports H264, H265, VP9 decoders. - Supports H264, H265 encoders. This driver comes with below features: - Centralized resource and memory management. - Centralized management of core and instance states. - Defines platform specific capabilities and features. As a results, it provides a single point of control to enable/disable a given feature depending on specific platform capabilities. - Handles hardware interdependent configurations. For a given configuration from client, the driver checks for hardware dependent configuration/s and updates the same. - Handles multiple complex video scenarios involving state transitions - DRC, Drain, Seek, back to back DRC, DRC during Drain sequence, DRC during Seek sequence. - Introduces a flexible way for driver to subscribe for any property with hardware. Hardware would inform driver with those subscribed property with any change in value. - Introduces performance (clock and bus) model based on new hardware architecture. - Introduces multi thread safe design to handle communication between client and hardware. - Adapts encoder quality improvements available in new hardware architecture. - Implements asynchronous communication with hardware to achieve better experience in low latency usecases. - Supports multi stage hardware architecture for encode/decode. - Output and capture planes are controlled independently. Thereby providing a way to reconfigure individual plane. - Hardware packetization layer supports synchronization between configuration packet and data packet. - Introduces a flexibility to receive a hardware response for a given command packet. - Native hardware support of LAST flag which is mandatory to align with port reconfiguration and DRAIN sequence as per V4L guidelines. - Native hardware support for drain sequence. Changes done since v1[1]: - Patches are created as logical split instead of file by file. - Extracted common functionality between venus and iris driver and placed them under common folder vcodec. - Flattened directory structure. - Restructured the code in a simplified layer architecture. - Implemented runtime PM, and dropped explicit power collapse thread in driver. - Moved to standard kernel log print api. - Simplified the bus and clock calculation logic. - Removed debug features like dma tracker, frame stats, debugfs. - Merged v4l2 and vidc layer as vidc in driver. - Removed separate probe for context bank. - Use of_device_get_match_data to get platform data. - Replaced complex macros with inline functions. - Migrated venus to iris names. - Addressed many other comments received on [1]. [1]: https://patchwork.kernel.org/project/linux-media/list/?series=770581 Patch 1, adds dt binding for iris driver. Patches 2-4, are introducing vcodec folder and moving common APIs like fw load/unload, buffer size calcualtions, read/write to shared queues to common files which are being used by both venus and iris drivers. This also moves the venus driver folder to vcodec folder. Patch 5, adds the maintainers for iris driver. Patch 6-29, adds core iris driver and enable decoder. Patch 30-34, enable encoder functionality in iris driver. Static tools like checkpatch, smatch, dt_binding_check, sparse and Coccinelle run successfully with this driver. The output of v4l2-compliance test looks like: v4l2-compliance SHA: not available, 64 bits Cannot open device /dev/vido0, exiting. root@qemuarm64:/usr/bin# ./v4l2-compliance -d /dev/video0 v4l2-compliance SHA: not available, 64 bits Compliance test for iris_driver device /dev/video0: Driver Info: Driver name : iris_driver Card type : iris_decoder Bus info : platform:iris_bus Driver version : 6.6.0 Capabilities : 0x84204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Device Capabilities Device Caps : 0x04204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK fail: ../../../v4l-utils-1.18.1/utils/v4l2-compliance/v4l2-compliance.cpp(724): !ok test for unlimited opens: FAIL Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 48 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK test Composing: OK test Scaling: OK Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK Buffer ioctls: testVIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK (Not Supported) test Requests: OK (Not Supported) Total for iris_driver device /dev/video0: 44, Succeeded: 43, Failed: 1, Warnings: 0 Compliance test for iris_driver device /dev/video1: Driver Info: Driver name : iris_driver Card type : iris_encoder Bus info : platform:iris_bus Driver version : 6.6.0 Capabilities : 0x84204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Device Capabilities Device Caps : 0x04204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video1 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK fail: ../../../v4l-utils-1.18.1/utils/v4l2-compliance/v4l2-compliance.cpp(724): !ok test for unlimited opens: FAIL Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 48 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK test Composing: OK test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK (Not Supported) test Requests: OK (Not Supported) Total for iris_driver device /dev/video1: 44, Succeeded: 43, Failed: 1, Warnings: 0 Dikshita Agarwal (27): media: introduce common helpers for video firmware handling media: introduce common helpers for queues handling media: introduce common helpers for buffer size calculation dt-bindings: media: Add sm8550 dt schema media: MAINTAINERS: Add Qualcomm Iris video accelerator driver media: iris: register video device to platform driver media: iris: initialize power resources media: iris: introduce state machine for iris core media: iris: initialize shared queues for host and firmware communication media: iris: add PIL functionality for video firmware media: iris: introduce packetization layer for creating HFI packets media: iris: add video processing unit(VPU) specific register handling media: iris: introduce platform specific capabilities for core and instance media: iris: add handling for interrupt service routine(ISR) invoked by hardware media: iris: implement iris v4l2_ctrl_ops and prepare capabilities media: iris: implement vb2_ops queue setup media: iris: implement HFI to queue and release buffers media: iris: add video hardware internal buffer count and size calculation media: iris: implement internal buffer management media: iris: introduce instance states media: iris: implement vb2 streaming ops on capture and output planes media: iris: implement vb2 ops for buf_queue and firmware response media: iris: register video encoder device to platform driver media: iris: add platform specific instance capabilities for encoder media: iris: implement iris v4l2 ioctl ops supported by encoder media: iris: add vb2 streaming and buffer ops for encoder media: iris: add power management for encoder Vikash Garodia (7): media: iris: implement iris v4l2 file ops media: iris: introduce and implement iris vb2 mem ops media: iris: implement iris v4l2 ioctl ops supported by decoder media: iris: subscribe input and output properties to firmware media: iris: subscribe src change and handle firmware responses media: iris: add instance sub states and implement DRC and Drain sequence media: iris: implement power management .../bindings/media/qcom,sm8550-iris.yaml | 177 ++ MAINTAINERS | 11 + drivers/media/platform/qcom/Kconfig | 3 +- drivers/media/platform/qcom/Makefile | 3 +- drivers/media/platform/qcom/vcodec/buffers.c | 103 ++ drivers/media/platform/qcom/vcodec/buffers.h | 15 + drivers/media/platform/qcom/vcodec/firmware.c | 147 ++ drivers/media/platform/qcom/vcodec/firmware.h | 21 + drivers/media/platform/qcom/vcodec/hfi_queue.c | 258 +++ drivers/media/platform/qcom/vcodec/hfi_queue.h | 129 ++ drivers/media/platform/qcom/vcodec/iris/Kconfig | 13 + drivers/media/platform/qcom/vcodec/iris/Makefile | 26 + .../media/platform/qcom/vcodec/iris/hfi_defines.h | 386 +++++ .../media/platform/qcom/vcodec/iris/iris_buffer.c | 833 +++++++++ .../media/platform/qcom/vcodec/iris/iris_buffer.h | 65 + .../media/platform/qcom/vcodec/iris/iris_common.h | 147 ++ .../media/platform/qcom/vcodec/iris/iris_core.c | 110 ++ .../media/platform/qcom/vcodec/iris/iris_core.h | 121 ++ .../media/platform/qcom/vcodec/iris/iris_ctrls.c | 1782 ++++++++++++++++++++ .../media/platform/qcom/vcodec/iris/iris_ctrls.h | 71 + .../media/platform/qcom/vcodec/iris/iris_helpers.c | 1099 ++++++++++++ .../media/platform/qcom/vcodec/iris/iris_helpers.h | 68 + drivers/media/platform/qcom/vcodec/iris/iris_hfi.c | 917 ++++++++++ drivers/media/platform/qcom/vcodec/iris/iris_hfi.h | 47 + .../platform/qcom/vcodec/iris/iris_hfi_packet.c | 729 ++++++++ .../platform/qcom/vcodec/iris/iris_hfi_packet.h | 118 ++ .../platform/qcom/vcodec/iris/iris_hfi_response.c | 1097 ++++++++++++ .../platform/qcom/vcodec/iris/iris_hfi_response.h | 20 + .../platform/qcom/vcodec/iris/iris_instance.h | 106 ++ .../media/platform/qcom/vcodec/iris/iris_power.c | 178 ++ .../media/platform/qcom/vcodec/iris/iris_power.h | 15 + .../media/platform/qcom/vcodec/iris/iris_probe.c | 357 ++++ .../media/platform/qcom/vcodec/iris/iris_state.c | 453 +++++ .../media/platform/qcom/vcodec/iris/iris_state.h | 78 + drivers/media/platform/qcom/vcodec/iris/iris_vb2.c | 457 +++++ drivers/media/platform/qcom/vcodec/iris/iris_vb2.h | 28 + .../media/platform/qcom/vcodec/iris/iris_vdec.c | 1211 +++++++++++++ .../media/platform/qcom/vcodec/iris/iris_vdec.h | 25 + .../media/platform/qcom/vcodec/iris/iris_venc.c | 948 +++++++++++ .../media/platform/qcom/vcodec/iris/iris_venc.h | 27 + .../media/platform/qcom/vcodec/iris/iris_vidc.c | 1419 ++++++++++++++++ .../media/platform/qcom/vcodec/iris/iris_vidc.h | 15 + .../platform/qcom/vcodec/iris/platform_common.c | 29 + .../platform/qcom/vcodec/iris/platform_common.h | 323 ++++ .../platform/qcom/vcodec/iris/platform_sm8550.c | 1190 +++++++++++++ .../media/platform/qcom/vcodec/iris/resources.c | 506 ++++++ .../media/platform/qcom/vcodec/iris/resources.h | 44 + .../media/platform/qcom/vcodec/iris/vpu_common.c | 158 ++ .../media/platform/qcom/vcodec/iris/vpu_common.h | 52 + .../media/platform/qcom/vcodec/iris/vpu_iris3.c | 568 +++++++ .../media/platform/qcom/vcodec/iris/vpu_iris3.h | 13 + .../platform/qcom/vcodec/iris/vpu_iris3_buffer.c | 395 +++++ .../platform/qcom/vcodec/iris/vpu_iris3_buffer.h | 1469 ++++++++++++++++ .../platform/qcom/vcodec/iris/vpu_iris3_power.c | 148 ++ .../platform/qcom/vcodec/iris/vpu_iris3_power.h | 13 + .../media/platform/qcom/{ => vcodec}/venus/Kconfig | 0 .../platform/qcom/{ => vcodec}/venus/Makefile | 5 +- .../media/platform/qcom/{ => vcodec}/venus/core.c | 102 +- .../media/platform/qcom/{ => vcodec}/venus/core.h | 0 .../media/platform/qcom/{ => vcodec}/venus/dbgfs.c | 0 .../media/platform/qcom/{ => vcodec}/venus/dbgfs.h | 0 .../platform/qcom/vcodec/venus/firmware_no_tz.c | 194 +++ .../platform/qcom/vcodec/venus/firmware_no_tz.h | 19 + .../platform/qcom/{ => vcodec}/venus/helpers.c | 172 +- .../platform/qcom/{ => vcodec}/venus/helpers.h | 2 +- .../media/platform/qcom/{ => vcodec}/venus/hfi.c | 0 .../media/platform/qcom/{ => vcodec}/venus/hfi.h | 0 .../platform/qcom/{ => vcodec}/venus/hfi_cmds.c | 0 .../platform/qcom/{ => vcodec}/venus/hfi_cmds.h | 0 .../platform/qcom/{ => vcodec}/venus/hfi_helper.h | 0 .../platform/qcom/{ => vcodec}/venus/hfi_msgs.c | 0 .../platform/qcom/{ => vcodec}/venus/hfi_msgs.h | 0 .../platform/qcom/{ => vcodec}/venus/hfi_parser.c | 0 .../platform/qcom/{ => vcodec}/venus/hfi_parser.h | 0 .../qcom/{ => vcodec}/venus/hfi_plat_bufs.h | 4 +- .../qcom/{ => vcodec}/venus/hfi_plat_bufs_v6.c | 10 +- .../qcom/{ => vcodec}/venus/hfi_platform.c | 0 .../qcom/{ => vcodec}/venus/hfi_platform.h | 0 .../qcom/{ => vcodec}/venus/hfi_platform_v4.c | 0 .../qcom/{ => vcodec}/venus/hfi_platform_v6.c | 0 .../platform/qcom/{ => vcodec}/venus/hfi_venus.c | 521 +----- .../platform/qcom/{ => vcodec}/venus/hfi_venus.h | 0 .../qcom/{ => vcodec}/venus/hfi_venus_io.h | 0 .../platform/qcom/{ => vcodec}/venus/pm_helpers.c | 0 .../platform/qcom/{ => vcodec}/venus/pm_helpers.h | 0 .../media/platform/qcom/{ => vcodec}/venus/vdec.c | 5 +- .../media/platform/qcom/{ => vcodec}/venus/vdec.h | 0 .../platform/qcom/{ => vcodec}/venus/vdec_ctrls.c | 0 .../media/platform/qcom/{ => vcodec}/venus/venc.c | 0 .../media/platform/qcom/{ => vcodec}/venus/venc.h | 0 .../platform/qcom/{ => vcodec}/venus/venc_ctrls.c | 0 drivers/media/platform/qcom/venus/firmware.c | 363 ---- drivers/media/platform/qcom/venus/firmware.h | 26 - 93 files changed, 19175 insertions(+), 989 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml create mode 100644 drivers/media/platform/qcom/vcodec/buffers.c create mode 100644 drivers/media/platform/qcom/vcodec/buffers.h create mode 100644 drivers/media/platform/qcom/vcodec/firmware.c create mode 100644 drivers/media/platform/qcom/vcodec/firmware.h create mode 100644 drivers/media/platform/qcom/vcodec/hfi_queue.c create mode 100644 drivers/media/platform/qcom/vcodec/hfi_queue.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/Kconfig create mode 100644 drivers/media/platform/qcom/vcodec/iris/Makefile create mode 100644 drivers/media/platform/qcom/vcodec/iris/hfi_defines.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_buffer.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_buffer.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_common.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_core.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_core.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_ctrls.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_ctrls.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_helpers.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_helpers.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_hfi.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_hfi.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_hfi_packet.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_hfi_packet.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_hfi_response.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_hfi_response.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_instance.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_power.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_power.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_probe.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_state.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_state.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vb2.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vb2.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vdec.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vdec.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_venc.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_venc.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vidc.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/iris_vidc.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/platform_common.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/platform_common.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/platform_sm8550.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/resources.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/resources.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/vpu_common.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/vpu_common.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/vpu_iris3.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/vpu_iris3.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/vpu_iris3_buffer.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/vpu_iris3_buffer.h create mode 100644 drivers/media/platform/qcom/vcodec/iris/vpu_iris3_power.c create mode 100644 drivers/media/platform/qcom/vcodec/iris/vpu_iris3_power.h rename drivers/media/platform/qcom/{ => vcodec}/venus/Kconfig (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/Makefile (75%) rename drivers/media/platform/qcom/{ => vcodec}/venus/core.c (91%) rename drivers/media/platform/qcom/{ => vcodec}/venus/core.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/dbgfs.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/dbgfs.h (100%) create mode 100644 drivers/media/platform/qcom/vcodec/venus/firmware_no_tz.c create mode 100644 drivers/media/platform/qcom/vcodec/venus/firmware_no_tz.h rename drivers/media/platform/qcom/{ => vcodec}/venus/helpers.c (90%) rename drivers/media/platform/qcom/{ => vcodec}/venus/helpers.h (98%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_cmds.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_cmds.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_helper.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_msgs.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_msgs.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_parser.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_parser.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_plat_bufs.h (94%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_plat_bufs_v6.c (99%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_platform.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_platform.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_platform_v4.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_platform_v6.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_venus.c (73%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_venus.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/hfi_venus_io.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/pm_helpers.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/pm_helpers.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/vdec.c (99%) rename drivers/media/platform/qcom/{ => vcodec}/venus/vdec.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/vdec_ctrls.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/venc.c (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/venc.h (100%) rename drivers/media/platform/qcom/{ => vcodec}/venus/venc_ctrls.c (100%) delete mode 100644 drivers/media/platform/qcom/venus/firmware.c delete mode 100644 drivers/media/platform/qcom/venus/firmware.h