On Mon 22 Aug 06:13 PDT 2016, Stanimir Varbanov wrote:
> This is the implementation of HFI. It is loaded with the
> responsibility to comunicate with the firmware through an
> interface commands and messages.
>
> - hfi.c has interface functions used by the core, decoder
> and encoder parts to comunicate with the firmware. For example
> there are functions for session and core initialisation.
>
I can't help feeling that the split between core.c and hfi.c is a
remnant of a vidc driver supporting both HFI and pre-HFI with the same
v4l code.
What do you think about merging vidc_core with hfi_core and vidc_inst
with hfi_inst? Both seems to be in a 1:1 relationship.
> - hfi_cmds has packetization operations which preparing
> packets to be send from host to firmware.
>
> - hfi_msgs takes care of messages sent from firmware to the
> host.
>
[..]
> diff --git a/drivers/media/platform/qcom/vidc/hfi_cmds.c b/drivers/media/platform/qcom/vidc/hfi_cmds.c
[..]
> +
> +static const struct hfi_packetization_ops hfi_default = {
> + .sys_init = pkt_sys_init,
> + .sys_pc_prep = pkt_sys_pc_prep,
> + .sys_idle_indicator = pkt_sys_idle_indicator,
> + .sys_power_control = pkt_sys_power_control,
> + .sys_set_resource = pkt_sys_set_resource,
> + .sys_release_resource = pkt_sys_unset_resource,
> + .sys_debug_config = pkt_sys_debug_config,
> + .sys_coverage_config = pkt_sys_coverage_config,
> + .sys_ping = pkt_sys_ping,
> + .sys_image_version = pkt_sys_image_version,
> + .ssr_cmd = pkt_ssr_cmd,
> + .session_init = pkt_session_init,
> + .session_cmd = pkt_session_cmd,
> + .session_set_buffers = pkt_session_set_buffers,
> + .session_release_buffers = pkt_session_release_buffers,
> + .session_etb_decoder = pkt_session_etb_decoder,
> + .session_etb_encoder = pkt_session_etb_encoder,
> + .session_ftb = pkt_session_ftb,
> + .session_parse_seq_header = pkt_session_parse_seq_header,
> + .session_get_seq_hdr = pkt_session_get_seq_hdr,
> + .session_flush = pkt_session_flush,
> + .session_get_property = pkt_session_get_property,
> + .session_set_property = pkt_session_set_property,
> +};
> +
> +static const struct hfi_packetization_ops *get_3xx_ops(void)
> +{
> + static struct hfi_packetization_ops hfi_3xx;
> +
> + hfi_3xx = hfi_default;
> + hfi_3xx.session_set_property = pkt_session_set_property_3xx;
> +
> + return &hfi_3xx;
> +}
> +
> +const struct hfi_packetization_ops *
> +hfi_get_pkt_ops(enum hfi_packetization_type type)
The only reasonable argument I can come up with for not just exposing
these as global functions would be that there are 23 of them... Can we
skip the jump table?
> +{
> + switch (type) {
> + case HFI_PACKETIZATION_LEGACY:
> + return &hfi_default;
> + case HFI_PACKETIZATION_3XX:
> + return get_3xx_ops();
> + }
> +
> + return NULL;
> +}
Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
