| Message ID | 20240827-iris_v3-v3-20-c5fdbbe65e70@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Qualcomm iris video decoder driver | expand |
On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote: > From: Vedang Nagar <quic_vnagar@quicinc.com> > > For hfi_gen2, subscribe for different bitstream parameters to > firmware to get notified for change in any of the subscribed > parameters. > > Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com> > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > drivers/media/platform/qcom/iris/iris_hfi_gen2.h | 6 + > .../platform/qcom/iris/iris_hfi_gen2_command.c | 179 +++++++++++++++++++++ > .../platform/qcom/iris/iris_hfi_gen2_defines.h | 9 ++ > .../platform/qcom/iris/iris_platform_common.h | 4 + > .../platform/qcom/iris/iris_platform_sm8550.c | 13 ++ > 5 files changed, 211 insertions(+) > > diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2.h > index 8170c1fef569..5fbbab844025 100644 > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2.h > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2.h > @@ -18,12 +18,18 @@ struct iris_core; > * > * @inst: pointer to iris_instance structure > * @packet: HFI packet > + * @ipsc_properties_set: boolean to set ipsc properties to fw > + * @opsc_properties_set: boolean to set opsc properties to fw > * @src_subcr_params: subscription params to fw on input port > + * @dst_subcr_params: subscription params to fw on output port > */ > struct iris_inst_hfi_gen2 { > struct iris_inst inst; > struct iris_hfi_header *packet; > + bool ipsc_properties_set; > + bool opsc_properties_set; > struct hfi_subscription_params src_subcr_params; > + struct hfi_subscription_params dst_subcr_params; > }; > > void iris_hfi_gen2_command_ops_init(struct iris_core *core); > 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 e50f00021f6d..791b535a3584 100644 > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c > @@ -472,6 +472,9 @@ static int iris_hfi_gen2_session_open(struct iris_inst *inst) > if (inst->state != IRIS_INST_DEINIT) > return -EALREADY; > > + inst_hfi_gen2->ipsc_properties_set = false; > + inst_hfi_gen2->opsc_properties_set = false; > + > inst_hfi_gen2->packet = kzalloc(4096, GFP_KERNEL); > if (!inst_hfi_gen2->packet) > return -ENOMEM; > @@ -536,9 +539,185 @@ static int iris_hfi_gen2_session_close(struct iris_inst *inst) > return ret; > } > > +static int iris_hfi_gen2_session_subscribe_mode(struct iris_inst *inst, > + u32 cmd, u32 plane, u32 payload_type, > + void *payload, u32 payload_size) > +{ > + struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); > + > + iris_hfi_gen2_packet_session_command(inst, > + cmd, > + (HFI_HOST_FLAGS_RESPONSE_REQUIRED | > + HFI_HOST_FLAGS_INTR_REQUIRED), > + iris_hfi_gen2_get_port(plane), > + inst->session_id, > + payload_type, > + payload, > + payload_size); > + > + return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet, > + inst_hfi_gen2->packet->size); > +} > + > +static int iris_hfi_gen2_subscribe_change_param(struct iris_inst *inst, u32 plane) > +{ > + struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); > + struct hfi_subscription_params subsc_params; > + u32 prop_type, payload_size, payload_type; > + struct iris_core *core = inst->core; > + const u32 *change_param = NULL; > + u32 change_param_size = 0; > + u32 payload[32] = {0}; > + u32 hfi_port = 0; > + int ret; > + u32 i; > + > + if ((V4L2_TYPE_IS_OUTPUT(plane) && inst_hfi_gen2->ipsc_properties_set) || > + (V4L2_TYPE_IS_CAPTURE(plane) && inst_hfi_gen2->opsc_properties_set)) { > + dev_err(core->dev, "invalid plane\n"); > + return 0; > + } > + > + change_param = core->iris_platform_data->input_config_params; > + change_param_size = core->iris_platform_data->input_config_params_size; > + > + if (!change_param || !change_param_size) > + return -EINVAL; That's an odd one, checking for zero but _not_ bounds checking chanage_param_size < (sizeof(payload)/sizeof(u32)) - 1 I'm not sure where inpug_config_param_size gets populated but I'd rather check that type of parameter - for exactly that reason - than the defensive coding done on your inputs elsewhere. TL;DR why do you trust change_param_size < your array size but not change_param_size >= 1 ? --- bod
On 9/24/2024 8:46 PM, Bryan O'Donoghue wrote: > On 27/08/2024 11:05, Dikshita Agarwal via B4 Relay wrote: >> From: Vedang Nagar <quic_vnagar@quicinc.com> >> >> For hfi_gen2, subscribe for different bitstream parameters to >> firmware to get notified for change in any of the subscribed >> parameters. >> >> Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com> >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >> --- >> drivers/media/platform/qcom/iris/iris_hfi_gen2.h | 6 + >> .../platform/qcom/iris/iris_hfi_gen2_command.c | 179 >> +++++++++++++++++++++ >> .../platform/qcom/iris/iris_hfi_gen2_defines.h | 9 ++ >> .../platform/qcom/iris/iris_platform_common.h | 4 + >> .../platform/qcom/iris/iris_platform_sm8550.c | 13 ++ >> 5 files changed, 211 insertions(+) >> >> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2.h >> b/drivers/media/platform/qcom/iris/iris_hfi_gen2.h >> index 8170c1fef569..5fbbab844025 100644 >> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2.h >> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2.h >> @@ -18,12 +18,18 @@ struct iris_core; >> * >> * @inst: pointer to iris_instance structure >> * @packet: HFI packet >> + * @ipsc_properties_set: boolean to set ipsc properties to fw >> + * @opsc_properties_set: boolean to set opsc properties to fw >> * @src_subcr_params: subscription params to fw on input port >> + * @dst_subcr_params: subscription params to fw on output port >> */ >> struct iris_inst_hfi_gen2 { >> struct iris_inst inst; >> struct iris_hfi_header *packet; >> + bool ipsc_properties_set; >> + bool opsc_properties_set; >> struct hfi_subscription_params src_subcr_params; >> + struct hfi_subscription_params dst_subcr_params; >> }; >> void iris_hfi_gen2_command_ops_init(struct iris_core *core); >> 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 e50f00021f6d..791b535a3584 100644 >> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c >> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c >> @@ -472,6 +472,9 @@ static int iris_hfi_gen2_session_open(struct >> iris_inst *inst) >> if (inst->state != IRIS_INST_DEINIT) >> return -EALREADY; >> + inst_hfi_gen2->ipsc_properties_set = false; >> + inst_hfi_gen2->opsc_properties_set = false; >> + >> inst_hfi_gen2->packet = kzalloc(4096, GFP_KERNEL); >> if (!inst_hfi_gen2->packet) >> return -ENOMEM; >> @@ -536,9 +539,185 @@ static int iris_hfi_gen2_session_close(struct >> iris_inst *inst) >> return ret; >> } >> +static int iris_hfi_gen2_session_subscribe_mode(struct iris_inst *inst, >> + u32 cmd, u32 plane, u32 payload_type, >> + void *payload, u32 payload_size) >> +{ >> + struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); >> + >> + iris_hfi_gen2_packet_session_command(inst, >> + cmd, >> + (HFI_HOST_FLAGS_RESPONSE_REQUIRED | >> + HFI_HOST_FLAGS_INTR_REQUIRED), >> + iris_hfi_gen2_get_port(plane), >> + inst->session_id, >> + payload_type, >> + payload, >> + payload_size); >> + >> + return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet, >> + inst_hfi_gen2->packet->size); >> +} >> + >> +static int iris_hfi_gen2_subscribe_change_param(struct iris_inst *inst, >> u32 plane) >> +{ >> + struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); >> + struct hfi_subscription_params subsc_params; >> + u32 prop_type, payload_size, payload_type; >> + struct iris_core *core = inst->core; >> + const u32 *change_param = NULL; >> + u32 change_param_size = 0; >> + u32 payload[32] = {0}; >> + u32 hfi_port = 0; >> + int ret; >> + u32 i; >> + >> + if ((V4L2_TYPE_IS_OUTPUT(plane) && >> inst_hfi_gen2->ipsc_properties_set) || >> + (V4L2_TYPE_IS_CAPTURE(plane) && >> inst_hfi_gen2->opsc_properties_set)) { >> + dev_err(core->dev, "invalid plane\n"); >> + return 0; >> + } >> + >> + change_param = core->iris_platform_data->input_config_params; >> + change_param_size = core->iris_platform_data->input_config_params_size; >> + >> + if (!change_param || !change_param_size) >> + return -EINVAL; > > That's an odd one, checking for zero but _not_ bounds checking > chanage_param_size < (sizeof(payload)/sizeof(u32)) - 1 > > I'm not sure where inpug_config_param_size gets populated but I'd rather > check that type of parameter - for exactly that reason - than the defensive > coding done on your inputs elsewhere. > > TL;DR why do you trust change_param_size < your array size but not > change_param_size >= 1 ? > These NULL checks here are actually not needed as we will make sure this is filled in platform data. Will remove these checks in next version and will see if bound check is required. > --- > bod
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2.h index 8170c1fef569..5fbbab844025 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2.h +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2.h @@ -18,12 +18,18 @@ struct iris_core; * * @inst: pointer to iris_instance structure * @packet: HFI packet + * @ipsc_properties_set: boolean to set ipsc properties to fw + * @opsc_properties_set: boolean to set opsc properties to fw * @src_subcr_params: subscription params to fw on input port + * @dst_subcr_params: subscription params to fw on output port */ struct iris_inst_hfi_gen2 { struct iris_inst inst; struct iris_hfi_header *packet; + bool ipsc_properties_set; + bool opsc_properties_set; struct hfi_subscription_params src_subcr_params; + struct hfi_subscription_params dst_subcr_params; }; void iris_hfi_gen2_command_ops_init(struct iris_core *core); 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 e50f00021f6d..791b535a3584 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c @@ -472,6 +472,9 @@ static int iris_hfi_gen2_session_open(struct iris_inst *inst) if (inst->state != IRIS_INST_DEINIT) return -EALREADY; + inst_hfi_gen2->ipsc_properties_set = false; + inst_hfi_gen2->opsc_properties_set = false; + inst_hfi_gen2->packet = kzalloc(4096, GFP_KERNEL); if (!inst_hfi_gen2->packet) return -ENOMEM; @@ -536,9 +539,185 @@ static int iris_hfi_gen2_session_close(struct iris_inst *inst) return ret; } +static int iris_hfi_gen2_session_subscribe_mode(struct iris_inst *inst, + u32 cmd, u32 plane, u32 payload_type, + void *payload, u32 payload_size) +{ + struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); + + iris_hfi_gen2_packet_session_command(inst, + cmd, + (HFI_HOST_FLAGS_RESPONSE_REQUIRED | + HFI_HOST_FLAGS_INTR_REQUIRED), + iris_hfi_gen2_get_port(plane), + inst->session_id, + payload_type, + payload, + payload_size); + + return iris_hfi_queue_cmd_write(inst->core, inst_hfi_gen2->packet, + inst_hfi_gen2->packet->size); +} + +static int iris_hfi_gen2_subscribe_change_param(struct iris_inst *inst, u32 plane) +{ + struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); + struct hfi_subscription_params subsc_params; + u32 prop_type, payload_size, payload_type; + struct iris_core *core = inst->core; + const u32 *change_param = NULL; + u32 change_param_size = 0; + u32 payload[32] = {0}; + u32 hfi_port = 0; + int ret; + u32 i; + + if ((V4L2_TYPE_IS_OUTPUT(plane) && inst_hfi_gen2->ipsc_properties_set) || + (V4L2_TYPE_IS_CAPTURE(plane) && inst_hfi_gen2->opsc_properties_set)) { + dev_err(core->dev, "invalid plane\n"); + return 0; + } + + change_param = core->iris_platform_data->input_config_params; + change_param_size = core->iris_platform_data->input_config_params_size; + + if (!change_param || !change_param_size) + return -EINVAL; + + payload[0] = HFI_MODE_PORT_SETTINGS_CHANGE; + + for (i = 0; i < change_param_size; i++) + payload[i + 1] = change_param[i]; + + ret = iris_hfi_gen2_session_subscribe_mode(inst, + HFI_CMD_SUBSCRIBE_MODE, + plane, + HFI_PAYLOAD_U32_ARRAY, + &payload[0], + ((change_param_size + 1) * sizeof(u32))); + if (ret) + return ret; + + if (V4L2_TYPE_IS_OUTPUT(plane)) { + inst_hfi_gen2->ipsc_properties_set = true; + } else { + hfi_port = iris_hfi_gen2_get_port(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); + memcpy(&inst_hfi_gen2->dst_subcr_params, + &inst_hfi_gen2->src_subcr_params, + sizeof(inst_hfi_gen2->src_subcr_params)); + subsc_params = inst_hfi_gen2->dst_subcr_params; + for (i = 0; i < change_param_size; i++) { + payload[0] = 0; + payload[1] = 0; + payload_size = 0; + payload_type = 0; + prop_type = change_param[i]; + switch (prop_type) { + case HFI_PROP_BITSTREAM_RESOLUTION: + payload[0] = subsc_params.bitstream_resolution; + payload_size = sizeof(u32); + payload_type = HFI_PAYLOAD_U32; + break; + case HFI_PROP_CROP_OFFSETS: + payload[0] = subsc_params.crop_offsets[0]; + payload[1] = subsc_params.crop_offsets[1]; + payload_size = sizeof(u64); + payload_type = HFI_PAYLOAD_64_PACKED; + break; + case HFI_PROP_CODED_FRAMES: + payload[0] = subsc_params.coded_frames; + payload_size = sizeof(u32); + payload_type = HFI_PAYLOAD_U32; + break; + case HFI_PROP_BUFFER_FW_MIN_OUTPUT_COUNT: + payload[0] = subsc_params.fw_min_count; + payload_size = sizeof(u32); + payload_type = HFI_PAYLOAD_U32; + break; + case HFI_PROP_PIC_ORDER_CNT_TYPE: + payload[0] = subsc_params.pic_order_cnt; + payload_size = sizeof(u32); + payload_type = HFI_PAYLOAD_U32; + break; + case HFI_PROP_SIGNAL_COLOR_INFO: + payload[0] = subsc_params.color_info; + payload_size = sizeof(u32); + payload_type = HFI_PAYLOAD_U32; + break; + case HFI_PROP_PROFILE: + payload[0] = subsc_params.profile; + payload_size = sizeof(u32); + payload_type = HFI_PAYLOAD_U32; + break; + case HFI_PROP_LEVEL: + payload[0] = subsc_params.level; + payload_size = sizeof(u32); + payload_type = HFI_PAYLOAD_U32; + break; + default: + prop_type = 0; + ret = -EINVAL; + break; + } + if (prop_type) { + ret = iris_hfi_gen2_session_set_property(inst, + prop_type, + HFI_HOST_FLAGS_NONE, + hfi_port, + payload_type, + &payload, + payload_size); + if (ret) + return ret; + } + } + inst_hfi_gen2->opsc_properties_set = true; + } + + return ret; +} + +static int iris_hfi_gen2_subscribe_property(struct iris_inst *inst, u32 plane) +{ + struct iris_core *core = inst->core; + const u32 *subcribe_prop = NULL; + u32 subscribe_prop_size = 0; + u32 payload[32] = {0}; + u32 i; + + payload[0] = HFI_MODE_PROPERTY; + + if (V4L2_TYPE_IS_OUTPUT(plane)) { + subscribe_prop_size = core->iris_platform_data->dec_input_prop_size; + subcribe_prop = core->iris_platform_data->dec_input_prop; + } else { + subscribe_prop_size = core->iris_platform_data->dec_output_prop_size; + subcribe_prop = core->iris_platform_data->dec_output_prop; + } + + for (i = 0; i < subscribe_prop_size; i++) + payload[i + 1] = subcribe_prop[i]; + + return iris_hfi_gen2_session_subscribe_mode(inst, + HFI_CMD_SUBSCRIBE_MODE, + plane, + HFI_PAYLOAD_U32_ARRAY, + &payload[0], + (subscribe_prop_size + 1) * sizeof(u32)); +} + static int iris_hfi_gen2_session_start(struct iris_inst *inst, u32 plane) { struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); + int ret = 0; + + ret = iris_hfi_gen2_subscribe_change_param(inst, plane); + if (ret) + return ret; + + ret = iris_hfi_gen2_subscribe_property(inst, plane); + if (ret) + return ret; iris_hfi_gen2_packet_session_command(inst, HFI_CMD_START, 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 b5b232d31ad8..92c44841c67d 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h @@ -17,6 +17,7 @@ #define HFI_CMD_CLOSE 0x01000004 #define HFI_CMD_START 0x01000005 #define HFI_CMD_STOP 0x01000006 +#define HFI_CMD_SUBSCRIBE_MODE 0x0100000B #define HFI_CMD_END 0x01FFFFFF #define HFI_BITMASK_FRAME_MBS_ONLY_FLAG 0x00000001 @@ -42,13 +43,16 @@ #define HFI_PROP_PIPE 0x0300010b #define HFI_PROP_LUMA_CHROMA_BIT_DEPTH 0x0300010f #define HFI_PROP_CODED_FRAMES 0x03000120 +#define HFI_PROP_CABAC_SESSION 0x03000121 #define HFI_PROP_BUFFER_HOST_MAX_COUNT 0x03000123 #define HFI_PROP_BUFFER_FW_MIN_OUTPUT_COUNT 0x03000124 #define HFI_PROP_PIC_ORDER_CNT_TYPE 0x03000128 #define HFI_PROP_QUALITY_MODE 0x03000148 #define HFI_PROP_SIGNAL_COLOR_INFO 0x03000155 +#define HFI_PROP_PICTURE_TYPE 0x03000162 #define HFI_PROP_DEC_DEFAULT_HEADER 0x03000168 #define HFI_PROP_DEC_START_FROM_RAP_FRAME 0x03000169 +#define HFI_PROP_NO_OUTPUT 0x0300016a #define HFI_PROP_END 0x03FFFFFF #define HFI_SESSION_ERROR_BEGIN 0x04000000 @@ -64,6 +68,11 @@ #define HFI_SYS_ERROR_WD_TIMEOUT 0x05000001 #define HFI_SYSTEM_ERROR_END 0x05FFFFFF +enum hfi_property_mode_type { + HFI_MODE_PORT_SETTINGS_CHANGE = 0x00000001, + HFI_MODE_PROPERTY = 0x00000002, +}; + enum hfi_color_format { HFI_COLOR_FMT_OPAQUE = 0, HFI_COLOR_FMT_NV12 = 1, diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h index 20422bdca6d4..5a01391845c6 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_common.h +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h @@ -159,6 +159,10 @@ struct iris_platform_data { unsigned int input_config_params_size; const u32 *output_config_params; unsigned int output_config_params_size; + const u32 *dec_input_prop; + unsigned int dec_input_prop_size; + const u32 *dec_output_prop; + unsigned int dec_output_prop_size; }; #endif diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c index 7be803ea867d..fccee5309d81 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_sm8550.c +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8550.c @@ -207,6 +207,15 @@ static const u32 sm8550_vdec_output_config_params[] = { HFI_PROP_LINEAR_STRIDE_SCANLINE, }; +static const u32 sm8550_vdec_subscribe_input_properties[] = { + HFI_PROP_NO_OUTPUT, +}; + +static const u32 sm8550_vdec_subscribe_output_properties[] = { + HFI_PROP_PICTURE_TYPE, + HFI_PROP_CABAC_SESSION, +}; + struct iris_platform_data sm8550_data = { .get_instance = iris_hfi_gen2_get_instance, .init_hfi_command_ops = iris_hfi_gen2_command_ops_init, @@ -244,4 +253,8 @@ struct iris_platform_data sm8550_data = { sm8550_vdec_output_config_params, .output_config_params_size = ARRAY_SIZE(sm8550_vdec_output_config_params), + .dec_input_prop = sm8550_vdec_subscribe_input_properties, + .dec_input_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_input_properties), + .dec_output_prop = sm8550_vdec_subscribe_output_properties, + .dec_output_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_output_properties), };