Re: [RFC PATCH 01/12] media: iris: Add HEVC and VP9 formats for decoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/03/2025 10:43, Dikshita Agarwal wrote:
Extend the decoder driver's supported formats to include HEVC (H.265)
and VP9. This change updates the format enumeration (VIDIOC_ENUM_FMT)
and allows setting these formats via VIDIOC_S_FMT.

Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx>
---
  .../qcom/iris/iris_hfi_gen1_command.c         | 18 ++++-
  .../qcom/iris/iris_hfi_gen1_defines.h         |  2 +
  .../qcom/iris/iris_hfi_gen2_command.c         | 16 ++++-
  .../qcom/iris/iris_hfi_gen2_defines.h         |  3 +
  .../media/platform/qcom/iris/iris_instance.h  |  2 +
  drivers/media/platform/qcom/iris/iris_vdec.c  | 69 +++++++++++++++++--
  drivers/media/platform/qcom/iris/iris_vdec.h  | 11 +++
  drivers/media/platform/qcom/iris/iris_vidc.c  |  3 -
  8 files changed, 113 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
index 64f887d9a17d..1e774b058ab9 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
@@ -26,6 +26,20 @@ static u32 iris_hfi_gen1_buf_type_from_driver(enum iris_buffer_type buffer_type)
  	}
  }
+static u32 iris_hfi_gen1_v4l2_to_codec_type(u32 pixfmt)
+{
+	switch (pixfmt) {
+	case V4L2_PIX_FMT_H264:
+		return HFI_VIDEO_CODEC_H264;
+	case V4L2_PIX_FMT_HEVC:
+		return HFI_VIDEO_CODEC_HEVC;
+	case V4L2_PIX_FMT_VP9:
+		return HFI_VIDEO_CODEC_VP9;
+	default:
+		return 0;
+	}

Unknown is 0 here - perhaps it should be a define.

+}
+
  static int iris_hfi_gen1_sys_init(struct iris_core *core)
  {
  	struct hfi_sys_init_pkt sys_init_pkt;
@@ -88,16 +102,18 @@ static int iris_hfi_gen1_sys_pc_prep(struct iris_core *core)
  static int iris_hfi_gen1_session_open(struct iris_inst *inst)
  {
  	struct hfi_session_open_pkt packet;
+	u32 codec;
  	int ret;
if (inst->state != IRIS_INST_DEINIT)
  		return -EALREADY;
+ codec = iris_hfi_gen1_v4l2_to_codec_type(inst->codec);


You can return an error from this function - suggest better error handling is

if (!codec)
	return -EINVAL; -ENO

or some other error value that makes more sense to you.

+static u32 iris_hfi_gen2_v4l2_to_codec_type(struct iris_inst *inst)
+{
+	switch (inst->codec) {
+	case V4L2_PIX_FMT_H264:
+		return HFI_CODEC_DECODE_AVC;
+	case V4L2_PIX_FMT_HEVC:
+		return HFI_CODEC_DECODE_HEVC;
+	case V4L2_PIX_FMT_VP9:
+		return HFI_CODEC_DECODE_VP9;
+	default:
+		return 0;

  static int iris_hfi_gen2_session_set_codec(struct iris_inst *inst)
  {
  	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
-	u32 codec = HFI_CODEC_DECODE_AVC;
+	u32 codec = iris_hfi_gen2_v4l2_to_codec_type(inst);

Same comment for gen2

iris_hfi_gen2_packet_session_property(inst,
  					      HFI_PROP_CODEC,
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
index 806f8bb7f505..2fcf7914b70f 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
@@ -104,6 +104,9 @@ enum hfi_color_format {
  enum hfi_codec_type {
  	HFI_CODEC_DECODE_AVC			= 1,
  	HFI_CODEC_ENCODE_AVC			= 2,
+	HFI_CODEC_DECODE_HEVC			= 3,
+	HFI_CODEC_ENCODE_HEVC			= 4,
+	HFI_CODEC_DECODE_VP9			= 5,
  };
enum hfi_picture_type {
diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
index caa3c6507006..d8f076936c2b 100644
--- a/drivers/media/platform/qcom/iris/iris_instance.h
+++ b/drivers/media/platform/qcom/iris/iris_instance.h
@@ -42,6 +42,7 @@
   * @sequence_out: a sequence counter for output queue
   * @tss: timestamp metadata
   * @metadata_idx: index for metadata buffer
+ * @codec: codec type
   */
struct iris_inst {
@@ -72,6 +73,7 @@ struct iris_inst {
  	u32				sequence_out;
  	struct iris_ts_metadata		tss[VIDEO_MAX_FRAME];
  	u32				metadata_idx;
+	u32				codec;
  };
#endif
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
index 4143acedfc57..cdcfe71f5b96 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.c
+++ b/drivers/media/platform/qcom/iris/iris_vdec.c
@@ -32,6 +32,7 @@ int iris_vdec_inst_init(struct iris_inst *inst)
  	f->fmt.pix_mp.width = DEFAULT_WIDTH;
  	f->fmt.pix_mp.height = DEFAULT_HEIGHT;
  	f->fmt.pix_mp.pixelformat = V4L2_PIX_FMT_H264;
+	inst->codec = f->fmt.pix_mp.pixelformat;
  	f->fmt.pix_mp.num_planes = 1;
  	f->fmt.pix_mp.plane_fmt[0].bytesperline = 0;
  	f->fmt.pix_mp.plane_fmt[0].sizeimage = iris_get_buffer_size(inst, BUF_INPUT);
@@ -67,14 +68,67 @@ void iris_vdec_inst_deinit(struct iris_inst *inst)
  	kfree(inst->fmt_src);
  }
+static const struct iris_fmt iris_vdec_formats[] = {
+	[IRIS_FMT_H264] = {
+		.pixfmt = V4L2_PIX_FMT_H264,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	},
+	[IRIS_FMT_HEVC] = {
+		.pixfmt = V4L2_PIX_FMT_HEVC,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	},
+	[IRIS_FMT_VP9] = {
+		.pixfmt = V4L2_PIX_FMT_VP9,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	},
+};
+
+static const struct iris_fmt *
+find_format(struct iris_inst *inst, u32 pixfmt, u32 type)
+{
+	const struct iris_fmt *fmt = iris_vdec_formats;
+	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
+	unsigned int i;

Slightly neater as a reverse christmas tree.

+
+	for (i = 0; i < size; i++) {
+		if (fmt[i].pixfmt == pixfmt)
+			break;
+	}
+
+	if (i == size || fmt[i].type != type)
+		return NULL;
+
+	return &fmt[i];
+}
+
+static const struct iris_fmt *
+find_format_by_index(struct iris_inst *inst, u32 index, u32 type)
+{
+	const struct iris_fmt *fmt = iris_vdec_formats;
+	unsigned int size = ARRAY_SIZE(iris_vdec_formats);
+
+	if (index >= size || fmt[index].type != type)
+		return NULL;
+
+	return &fmt[index];
+}
+
  int iris_vdec_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f)
  {
+	const struct iris_fmt *fmt;
+
  	switch (f->type) {
  	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
-		f->pixelformat = V4L2_PIX_FMT_H264;
+		fmt = find_format_by_index(inst, f->index, f->type);
+		if (!fmt)
+			return -EINVAL;
+
+		f->pixelformat = fmt->pixfmt;
  		f->flags = V4L2_FMT_FLAG_COMPRESSED | V4L2_FMT_FLAG_DYN_RESOLUTION;
  		break;
  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		if (f->index)
+			return -EINVAL;
  		f->pixelformat = V4L2_PIX_FMT_NV12;
  		break;
  	default:
@@ -88,13 +142,15 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
  {
  	struct v4l2_pix_format_mplane *pixmp = &f->fmt.pix_mp;
  	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
+	const struct iris_fmt *fmt;
  	struct v4l2_format *f_inst;
  	struct vb2_queue *src_q;
memset(pixmp->reserved, 0, sizeof(pixmp->reserved));
+	fmt = find_format(inst, pixmp->pixelformat, f->type);
  	switch (f->type) {
  	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
-		if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_H264) {
+		if (!fmt) {
  			f_inst = inst->fmt_src;
  			f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
  			f->fmt.pix_mp.height = f_inst->fmt.pix_mp.height;
@@ -102,7 +158,7 @@ int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f)
  		}
  		break;
  	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
-		if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_NV12) {
+		if (!fmt) {
  			f_inst = inst->fmt_dst;
  			f->fmt.pix_mp.pixelformat = f_inst->fmt.pix_mp.pixelformat;
  			f->fmt.pix_mp.width = f_inst->fmt.pix_mp.width;
@@ -145,13 +201,14 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
switch (f->type) {
  	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
-		if (f->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_H264)
+		if (!(find_format(inst, f->fmt.pix_mp.pixelformat, f->type)))
  			return -EINVAL;
fmt = inst->fmt_src;
  		fmt->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
-
-		codec_align = DEFAULT_CODEC_ALIGNMENT;
+		fmt->fmt.pix_mp.pixelformat = f->fmt.pix_mp.pixelformat;
+		inst->codec = fmt->fmt.pix_mp.pixelformat;
+		codec_align = inst->codec == V4L2_PIX_FMT_HEVC ? 32 : 16;

For preference I'd choose a default assignment and then an if for whatever you choose as non-default.

codec_align = 16;
if (inst->codec == V4L2_PIX_FMT_HEVC)
	codec_align = 32;

  		fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, codec_align);
  		fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, codec_align);
  		fmt->fmt.pix_mp.num_planes = 1;
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.h b/drivers/media/platform/qcom/iris/iris_vdec.h
index b24932dc511a..cd7aab66dc7c 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.h
+++ b/drivers/media/platform/qcom/iris/iris_vdec.h
@@ -8,6 +8,17 @@
struct iris_inst; +enum iris_fmt_type {
+	IRIS_FMT_H264,

I persoanlly like to init enums = 0,


+	IRIS_FMT_HEVC,
+	IRIS_FMT_VP9,
+};
+
+struct iris_fmt {
+	u32 pixfmt;
+	u32 type;
+};
+
  int iris_vdec_inst_init(struct iris_inst *inst);
  void iris_vdec_inst_deinit(struct iris_inst *inst);
  int iris_vdec_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f);
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
index ca0f4e310f77..6a6afa15b647 100644
--- a/drivers/media/platform/qcom/iris/iris_vidc.c
+++ b/drivers/media/platform/qcom/iris/iris_vidc.c
@@ -249,9 +249,6 @@ static int iris_enum_fmt(struct file *filp, void *fh, struct v4l2_fmtdesc *f)
  {
  	struct iris_inst *inst = iris_get_inst(filp, NULL);
- if (f->index)
-		return -EINVAL;
-
  	return iris_vdec_enum_fmt(inst, f);
  }

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux