mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
To: Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Vikash Garodia <vikash.garodia@oss.qualcomm.com>,
	Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>,
	Abhinav Kumar <abhinav.kumar@linux.dev>,
	Bryan O'Donoghue <bod@kernel.org>
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, kernel test robot <lkp@intel.com>
Subject: Re: [PATCH v7 5/5] media: iris: Add BUF_ROIMB_DELTAQP metadata buffer for ROI delta QP
Date: Wed, 30 Sep 2026 16:28:24 +0530	[thread overview]
Message-ID: <8e350ba2-0eb7-7f69-e36c-7c0294e9c564@oss.qualcomm.com> (raw)
In-Reply-To: <20260924-enc_roi_enable-v7-5-6fa7413af23c@oss.qualcomm.com>


On 9/25/2026 5:58 AM, Deepa Guthyappa Madivalara wrote:
> - Introduce BUF_ROIMB_DELTAQP as a new internal buffer type to carry
>   per-macroblock ROI delta QP data to the firmware. This buffer is
>   allocated on demand when the application sets ROI parameters via
>   V4L2_CID_MPEG_VIDEO_ROI_MB_DELTA_QP, and is queued alongside each
>   raw input buffer as an HFI_BUFFER_METADATA packet on HFI_PORT_RAW.
>
> - The metadata buffer layout follows the firmware-defined metabuf_header
>   and metapayload_header structures. The s8 per-MB delta QP values
>   supplied by userspace are widened to s16 as required by the firmware.
>
> Signed-off-by: Deepa Guthyappa Madivalara <deepa.madivalara@oss.qualcomm.com>
> ---
>  drivers/media/platform/qcom/iris/iris_buffer.c     | 114 +++++++++++++++++++++
>  drivers/media/platform/qcom/iris/iris_buffer.h     |   4 +
>  drivers/media/platform/qcom/iris/iris_ctrls.c      |  37 +++++++
>  drivers/media/platform/qcom/iris/iris_ctrls.h      |   1 +
>  drivers/media/platform/qcom/iris/iris_hfi_gen2.c   |   1 +
>  .../platform/qcom/iris/iris_hfi_gen2_command.c     |  24 +++++
>  .../platform/qcom/iris/iris_hfi_gen2_response.c    |   2 +
>  drivers/media/platform/qcom/iris/iris_vidc.c       |   2 +
>  8 files changed, 185 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index eb8de60c1177f5e1ab83b90a3c8c80e0b4d1f02e..a7f1d74360d6a490959b27936fa5bbc61a7d9358 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -5,11 +5,13 @@
>  
>  #include <media/v4l2-event.h>
>  #include <media/v4l2-mem2mem.h>
> +#include <linux/slab.h>
>  
>  #include "iris_buffer.h"
>  #include "iris_instance.h"
>  #include "iris_power.h"
>  #include "iris_vpu_buffer.h"
> +#include "iris_hfi_gen2_defines.h"
>  
>  #define PIXELS_4K 4096
>  #define MAX_WIDTH 4096
> @@ -705,6 +707,20 @@ int iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 plane)
>  	return iris_destroy_internal_buffers(inst, plane, false);
>  }
>  
> +int iris_destroy_roi_metadata_buffers(struct iris_inst *inst)
> +{
> +	struct iris_buffer *buf, *next;
> +	struct iris_buffers *buffers;
> +
> +	if (inst->domain == ENCODER) {
> +		buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
> +		list_for_each_entry_safe(buf, next, &buffers->list, list)
> +			iris_destroy_internal_buffer(inst, buf);
> +	}
> +
> +	return 0;
> +}
> +
>  static int iris_release_internal_buffers(struct iris_inst *inst,
>  					 enum iris_buffer_type buffer_type)
>  {
> @@ -928,3 +944,101 @@ int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf)
>  
>  	return 0;
>  }
> +
> +static int iris_fill_roi_data(struct iris_inst *inst, struct iris_buffer *buffer)
> +{
> +	s8 *p_array = (s8 *)inst->fw_caps[ROI_PARAMS].p_array;
> +	u32 payload_offset, lcu_width, lcu_height, row_size;
> +	u32 array_size = inst->fw_caps[ROI_PARAMS].elems;
> +	struct metabuf_header *mbuf_hdr = buffer->kvaddr;
> +	struct metapayload_header *mbuf_payload_hdr;
> +	struct v4l2_format *f = inst->fmt_src;
> +	s16 *p_16, *p_temp;
> +
> +	memset(mbuf_hdr, 0, sizeof(struct metabuf_header));
> +	mbuf_hdr->count = 1;
> +	mbuf_hdr->size = sizeof(struct metabuf_header) +
> +			 sizeof(struct metapayload_header);
> +	mbuf_hdr->version = BIT(16);
> +	mbuf_hdr++;
> +	mbuf_payload_hdr = (struct metapayload_header *)(mbuf_hdr);
> +	payload_offset = sizeof(struct metabuf_header) +
> +			 sizeof(struct metapayload_header);
> +
> +	memset(mbuf_payload_hdr, 0, sizeof(struct metapayload_header));
> +	mbuf_payload_hdr->type = HFI_PROP_ROI_INFO;
> +	mbuf_payload_hdr->size = array_size * sizeof(s16);
> +	mbuf_payload_hdr->version = 1 << 16;
> +	mbuf_payload_hdr->offset = ALIGN(payload_offset, (u32)256);
> +	mbuf_payload_hdr->flags = 0;
> +
> +	if (inst->codec == V4L2_PIX_FMT_HEVC) {
> +		lcu_width = (f->fmt.pix_mp.width + 31) >> 5;
> +		lcu_height = (f->fmt.pix_mp.height + 31) >> 5;
> +	} else {
> +		lcu_width = (f->fmt.pix_mp.width + 15) >> 4;
> +		lcu_height = (f->fmt.pix_mp.height + 15) >> 4;
> +	}
> +	/* Firmware expects each row to align to 8 LCU */
> +	row_size = (((lcu_width + 7) >> 3) << 3);
> +
> +	/* Firmware expects 2bytes of delta_Qp, int16_t */
> +	p_16 = buffer->kvaddr + mbuf_payload_hdr->offset;
> +	for (int j = 0; j < lcu_height; j++) {
> +		p_temp = p_16 + j * row_size;
> +		for (int i = 0; i < lcu_width; i++)
> +			*p_temp++ = (1 << 11) | (((u8)p_array[j * lcu_width + i]
> +						   & 0x3F) << 4);
> +	}
> +
> +	return 0;
> +}
> +
> +int iris_hfi_gen2_session_alloc_roi_metadata_buffer(struct iris_inst *inst)
> +{
> +	struct iris_buffers *buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
> +	struct iris_core *core = inst->core;
> +	struct iris_buffer *buffer, *first_buffer, *next;
> +	bool found = false;
> +	int ret = 0;
> +
> +	if (!buffers->size)
> +		return 0;
> +
> +	list_for_each_entry_safe(buffer, next, &buffers->list, list) {
> +		if (buffer->attr & BUF_ATTR_DEQUEUED) {
> +			buffer->attr &= ~BUF_ATTR_DEQUEUED;
> +			list_move(&buffer->list, &buffers->list);
> +			found = true;
> +			break;
> +		}
> +	}
> +	if (!found) {
> +		buffer = kzalloc_obj(*buffer);
> +		if (!buffer)
> +			return -ENOMEM;
> +
> +		INIT_LIST_HEAD(&buffer->list);
> +		buffer->type = BUF_ROIMB_DELTAQP;
> +		buffer->index++;
> +		buffer->buffer_size = buffers->size;
> +		buffer->dma_attrs = DMA_ATTR_WRITE_COMBINE;
> +
> +		buffer->kvaddr = dma_alloc_attrs(core->dev, buffer->buffer_size,
> +						 &buffer->device_addr, GFP_KERNEL,
> +						 buffer->dma_attrs);
> +
> +		if (!buffer->kvaddr) {
> +			kfree(buffer);
> +			return -ENOMEM;
> +		}
> +		list_add(&buffer->list, &buffers->list);
> +	}
> +
> +	first_buffer = list_first_entry(&buffers->list, struct iris_buffer, list);
> +	ret = iris_fill_roi_data(inst, first_buffer);
> +	if (ret)
> +		return ret;
> +
> +	return ret;
> +}
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h
> index ee2d24bb69c57220b0a735d9b4aae4434a33daf6..df9d018493f3ec37fd5e6b0ba049dba0b5ae76e9 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.h
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.h
> @@ -28,6 +28,7 @@ struct iris_inst;
>   * @BUF_SCRATCH_2: buffer to store encoding context data for HW
>   * @BUF_VPSS: buffer to store VPSS context data for HW
>   * @BUF_PARTIAL: buffer for AV1 IBC data
> + * @BUF_ROIMB_DELTAQP: metadata buffer for ROI MB DeltaQp
>   * @BUF_TYPE_MAX: max buffer types
>   */
>  enum iris_buffer_type {
> @@ -44,6 +45,7 @@ enum iris_buffer_type {
>  	BUF_SCRATCH_2,
>  	BUF_VPSS,
>  	BUF_PARTIAL,
> +	BUF_ROIMB_DELTAQP,
>  	BUF_TYPE_MAX,
>  };
>  
> @@ -139,5 +141,7 @@ int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf);
>  int iris_queue_deferred_buffers(struct iris_inst *inst, enum iris_buffer_type buf_type);
>  int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf);
>  void iris_vb2_queue_error(struct iris_inst *inst);
> +int iris_hfi_gen2_session_alloc_roi_metadata_buffer(struct iris_inst *inst);
> +int iris_destroy_roi_metadata_buffers(struct iris_inst *inst);
>  
>  #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c
> index 1ce8eccdedbdeb840833bd92edb3f59dd6962d7f..2ab20cacbc2b79ac4cf28eda88b15cae332c4160 100644
> --- a/drivers/media/platform/qcom/iris/iris_ctrls.c
> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c
> @@ -1624,3 +1624,40 @@ int iris_set_metadata_delivery(struct iris_inst *inst, u32 plane)
>  
>  	return ret;
>  }
> +
> +int iris_set_roi_params(struct iris_inst *inst, u32 plane)
> +{
> +	struct iris_buffers *buffers = &inst->buffers[BUF_ROIMB_DELTAQP];
> +	u32 height = inst->fmt_src->fmt.pix_mp.height;
> +	u32 width = inst->fmt_src->fmt.pix_mp.width;
> +	u32 metadata_header_bytes = 256;
> +	u32 lcu_height = 0;
> +	u32 lcu_width = 0;
> +	u32 lcu_size = 16;
> +	u32 n_shift = 0;
> +	u32 size = 0;
> +	int ret = 0;
> +
> +	if (!inst->fw_caps[ROI_PARAMS].p_array)
> +		return -EINVAL;
> +
> +	if (inst->codec == V4L2_PIX_FMT_HEVC)
> +		lcu_size = 32;
> +
> +	while (lcu_size && !(lcu_size & 0x1)) {
> +		n_shift++;
> +		lcu_size = lcu_size >> 1;

Since lcu_size can only be 16 or 32 here, is the loop needed?
n_shift could be derived using ilog2(lcu_size), and the lcu_size reassignment
below could also be avoided.

> +	}
> +
> +	lcu_size = inst->codec == V4L2_PIX_FMT_HEVC ? 32 : 16;
> +	lcu_width = (width + (lcu_size - 1)) >> n_shift;
> +	lcu_height = (height + (lcu_size - 1)) >> n_shift;
> +	size = (((lcu_width + 7) >> 3) << 3) * lcu_height * 2;
> +	size += metadata_header_bytes;
> +	buffers->size = ALIGN(size, 4096);
> +	ret = iris_hfi_gen2_session_alloc_roi_metadata_buffer(inst);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

return iris_hfi_gen2_session_alloc_roi_metadata_buffer(inst);

> +}
> diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.h b/drivers/media/platform/qcom/iris/iris_ctrls.h
> index 9957b69366f50310338a0c1470d9744592655c33..7587dfec052a320863889eb15203416b9c527a40 100644
> --- a/drivers/media/platform/qcom/iris/iris_ctrls.h
> +++ b/drivers/media/platform/qcom/iris/iris_ctrls.h
> @@ -51,5 +51,6 @@ int iris_set_req_sync_frame(struct iris_inst *inst, enum platform_inst_fw_cap_ty
>  int iris_set_time_delta_based_rc(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id);
>  int iris_set_properties(struct iris_inst *inst, u32 plane);
>  int iris_set_metadata_delivery(struct iris_inst *inst, u32 plane);
> +int iris_set_roi_params(struct iris_inst *inst, u32 plane);
>  
>  #endif
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
> index d628bc6748438b19dac01cd11cb9f8da0b8500af..4d40d6733e5877a302ce63fb0b4a61b8f284e6dc 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2.c
> @@ -1789,6 +1789,7 @@ static const struct platform_inst_fw_cap inst_fw_cap_sm8550_enc[] = {
>  		.hfi_id = HFI_PROP_ROI_INFO,
>  		.flags = CAP_FLAG_INPUT_PORT | CAP_FLAG_CUSTOM |
>  			 CAP_FLAG_DYNAMIC_ALLOWED,
> +		.set = iris_set_roi_params,
>  	},
>  	{
>  		.cap_id = MB_SIZE,
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> index cf88dbe11e8e9faef826e661d8e9909fd7f16b94..a1b0c4982f0d789a807f22a1dffdf61decd20d28 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> @@ -134,6 +134,7 @@ static u32 iris_hfi_gen2_get_port_from_buf_type(struct iris_inst *inst,
>  		switch (buffer_type) {
>  		case BUF_INPUT:
>  		case BUF_VPSS:
> +		case BUF_ROIMB_DELTAQP:
>  			return HFI_PORT_RAW;
>  		case BUF_OUTPUT:
>  		case BUF_BIN:
> @@ -1267,6 +1268,8 @@ static u32 iris_hfi_gen2_buf_type_from_driver(u32 domain, enum iris_buffer_type
>  		return HFI_BUFFER_VPSS;
>  	case BUF_PARTIAL:
>  		return HFI_BUFFER_PARTIAL_DATA;
> +	case BUF_ROIMB_DELTAQP:
> +		return HFI_BUFFER_METADATA;
>  	default:
>  		return 0;
>  	}
> @@ -1328,6 +1331,7 @@ static struct iris_buffer *iris_queue_metadata_buffers(struct iris_inst *inst,
>  static int iris_hfi_gen2_session_queue_buffer(struct iris_inst *inst, struct iris_buffer *buffer)
>  {
>  	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
> +	struct iris_hfi_buffer hfi_meta_buffer;
>  	struct iris_hfi_buffer hfi_buffer;
>  	u32 port;
>  	int ret;
> @@ -1349,6 +1353,26 @@ static int iris_hfi_gen2_session_queue_buffer(struct iris_inst *inst, struct iri
>  					     &hfi_buffer,
>  					     sizeof(hfi_buffer));
>  
> +	/* check if any metadata buffer is available not queued, queue it */
> +	if (port == HFI_PORT_RAW) {
> +		buffer = iris_queue_metadata_buffers(inst, BUF_ROIMB_DELTAQP, buffer->index);
> +		if (buffer) {
> +			iris_hfi_gen2_get_buffer(inst->domain, buffer, &hfi_meta_buffer);
> +			port = iris_hfi_gen2_get_port_from_buf_type(inst, buffer->type);
> +			iris_hfi_gen2_create_packet(inst_hfi_gen2->packet,
> +						    HFI_CMD_BUFFER,
> +						    HFI_HOST_FLAGS_INTR_REQUIRED,
> +						    HFI_PAYLOAD_STRUCTURE,
> +						    port,
> +						    inst->core->packet_id++,
> +						    &hfi_meta_buffer,
> +						    sizeof(hfi_meta_buffer));
> +
> +			buffer->attr |= BUF_ATTR_QUEUED;
> +			buffer->attr &= ~BUF_ATTR_DEQUEUED;

If iris_queue_buffer() is the only caller, is this needed?
The caller already updates QUEUED attribute on successful return.

> +		}
> +	}
> +
>  	return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet,
>  					inst_hfi_gen2->packet->size);
>  }
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> index 84b629b957442c024dccc18b354ac9a7e541abaa..f63e0a7723e8583da818c2b004e367c1dd3d94f7 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
> @@ -58,6 +58,8 @@ static u32 iris_hfi_gen2_buf_type_to_driver(struct iris_inst *inst,
>  		return BUF_PARTIAL;
>  	case HFI_BUFFER_VPSS:
>  		return BUF_VPSS;
> +	case HFI_BUFFER_METADATA:
> +		return BUF_ROIMB_DELTAQP;
>  	default:
>  		return 0;
>  	}
> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
> index fcbc60016beec693f2ce27927a09a2d51494bc38..fae3d113a9dc990f06167ee0b7fb8fab24a0552c 100644
> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
> @@ -180,6 +180,7 @@ int iris_open(struct file *filp)
>  	INIT_LIST_HEAD(&inst->buffers[BUF_SCRATCH_2].list);
>  	INIT_LIST_HEAD(&inst->buffers[BUF_VPSS].list);
>  	INIT_LIST_HEAD(&inst->buffers[BUF_PARTIAL].list);
> +	INIT_LIST_HEAD(&inst->buffers[BUF_ROIMB_DELTAQP].list);
>  	init_completion(&inst->completion);
>  	init_completion(&inst->flush_completion);
>  
> @@ -299,6 +300,7 @@ int iris_close(struct file *filp)
>  	iris_destroy_all_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
>  	iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>  	iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	iris_destroy_roi_metadata_buffers(inst);
>  	iris_remove_session(inst);
>  	mutex_unlock(&inst->lock);
>  	mutex_destroy(&inst->ctx_q_lock);
>

      reply	other threads:[~2026-09-30 10:58 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25  0:28 [PATCH v7 0/5] Implement Region of Interest(ROI) support Deepa Guthyappa Madivalara
2026-09-25  0:28 ` [PATCH v7 1/5] media: v4l2-core: add new control type V4L2_CTRL_TYPE_S8 Deepa Guthyappa Madivalara
2026-09-30  7:14   ` Hans Verkuil
2026-09-30  9:53   ` Vishnu Reddy
2026-09-25  0:28 ` [PATCH v7 2/5] media: uapi: Introduce new control for video encoder ROI Deepa Guthyappa Madivalara
2026-09-28 14:32   ` Nicolas Dufresne
2026-09-28 21:11     ` Deepa Guthyappa Madivalara
2026-09-30  7:15   ` Hans Verkuil
2026-09-25  0:28 ` [PATCH v7 3/5] media: iris: Add ROI delta QP control support for HFI Gen2 encoders Deepa Guthyappa Madivalara
2026-09-25  0:28 ` [PATCH v7 4/5] media: iris: Add HFI metadata buffer delivery support for " Deepa Guthyappa Madivalara
2026-09-30 10:32   ` Vishnu Reddy
2026-09-25  0:28 ` [PATCH v7 5/5] media: iris: Add BUF_ROIMB_DELTAQP metadata buffer for ROI delta QP Deepa Guthyappa Madivalara
2026-09-30 10:58   ` Vishnu Reddy [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8e350ba2-0eb7-7f69-e36c-7c0294e9c564@oss.qualcomm.com \
    --to=busanna.reddy@oss.qualcomm.com \
    --cc=abhinav.kumar@linux.dev \
    --cc=bod@kernel.org \
    --cc=deepa.madivalara@oss.qualcomm.com \
    --cc=dikshita.agarwal@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mchehab@kernel.org \
    --cc=vikash.garodia@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®