Andreas,
On 11/23/23 5:33 PM, Andreas Helbech Kleist wrote:
> Hi,
>
> On Tue, 2023-10-24 at 19:29 +0800, bingbu.cao@xxxxxxxxx wrote:
>> From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>>
>> Syscom is an inter-process(or) communication mechanism between an IPU
>> and host. Syscom uses message queues for message exchange between IPU
>> and host. Each message queue has its consumer and producer, host
>> queue
>> messages to firmware as the producer and then firmware to dequeue the
>> messages as consumer and vice versa. IPU and host use shared
>> registers
>> or memory to reside the read and write indices which are updated by
>> consumer and producer.
>>
>> Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>> ---
>> drivers/media/pci/intel/ipu6/ipu6-fw-com.c | 411
>> +++++++++++++++++++++
>> drivers/media/pci/intel/ipu6/ipu6-fw-com.h | 47 +++
>> 2 files changed, 458 insertions(+)
>> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-com.c
>> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-com.h
> ...
>> +struct ipu6_fw_com_context {
>> + struct ipu6_bus_device *adev;
>> + void __iomem *dmem_addr;
>> + int (*cell_ready)(struct ipu6_bus_device *adev);
>> + void (*cell_start)(struct ipu6_bus_device *adev);
>
> Why are cell_ready and cell_start function pointers? They seem to
> always be set to query_sp and start_sp.
>
>> +
>> + void *dma_buffer;
>> + dma_addr_t dma_addr;
>> + unsigned int dma_size;
>> + unsigned long attrs;
>> +
>> + struct ipu6_fw_sys_queue *input_queue; /* array of host to
>> SP queues */
>> + struct ipu6_fw_sys_queue *output_queue; /* array of SP to
>> host */
>> +
>> + u32 config_vied_addr;
>> +
>> + unsigned int buttress_boot_offset;
>> + void __iomem *base_addr;
>> +};
>> +
>> +#define FW_COM_WR_REG 0
>> +#define FW_COM_RD_REG 4
>> +
>> +#define REGMEM_OFFSET 0
>> +#define TUNIT_MAGIC_PATTERN 0x5a5a5a5a
>> +
>> +enum regmem_id {
>> + /* pass pkg_dir address to SPC in non-secure mode */
>> + PKG_DIR_ADDR_REG = 0,
>> + /* Tunit CFG blob for secure - provided by host.*/
>> + TUNIT_CFG_DWR_REG = 1,
>> + /* syscom commands - modified by the host */
>> + SYSCOM_COMMAND_REG = 2,
>> + /* Store interrupt status - updated by SP */
>> + SYSCOM_IRQ_REG = 3,
>> + /* first syscom queue pointer register */
>> + SYSCOM_QPR_BASE_REG = 4
>> +};
>> +
>> +enum message_direction {
>> + DIR_RECV = 0,
>> + DIR_SEND
>> +};
>
> Not used?
Ack, will remove.
>
>
> ...
>> +struct ipu6_fw_com_cfg {
>> + unsigned int num_input_queues;
>> + unsigned int num_output_queues;
>> + struct ipu6_fw_syscom_queue_config *input;
>> + struct ipu6_fw_syscom_queue_config *output;
>> +
>> + unsigned int dmem_addr;
>> +
>> + /* firmware-specific configuration data */
>> + void *specific_addr;
>> + unsigned int specific_size;
>> + int (*cell_ready)(struct ipu6_bus_device *adev);
>> + void (*cell_start)(struct ipu6_bus_device *adev);
>> +
>> + unsigned int buttress_boot_offset;
>
> This seems to always be 0 (set by ipu6-fw-isys.c), seems to be trivial
> to remove.
All these fields are all used to extend for psys driver.
>
> /Andreas
>
--
Best regards,
Bingbu Cao
