Hi,
On 12.01.2023 19:18, Jeffrey Hugo wrote:
> On 1/9/2023 5:23 AM, Jacek Lawrynowicz wrote:
>> The IPC driver is used to send and receive messages to/from firmware
>> running on the VPU.
>>
>> The only supported IPC message format is Job Submission Model (JSM)
>> defined in vpu_jsm_api.h header.
>>
>> Co-developed-by: Andrzej Kacprowski <andrzej.kacprow...@linux.intel.com>
>> Signed-off-by: Andrzej Kacprowski <andrzej.kacprow...@linux.intel.com>
>> Co-developed-by: Krystian Pradzynski <krystian.pradzyn...@linux.intel.com>
>> Signed-off-by: Krystian Pradzynski <krystian.pradzyn...@linux.intel.com>
>> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynow...@linux.intel.com>
>
> Reviewed-by: Jeffrey Hugo <quic_jh...@quicinc.com>
>
>> +int ivpu_ipc_irq_handler(struct ivpu_device *vdev)
>> +{
>> + struct ivpu_ipc_info *ipc = vdev->ipc;
>> + struct ivpu_ipc_consumer *cons;
>> + struct ivpu_ipc_hdr *ipc_hdr;
>> + struct vpu_jsm_msg *jsm_msg;
>> + unsigned long flags;
>> + bool dispatched;
>> + u32 vpu_addr;
>> +
>> + /* Driver needs to purge all messages from IPC FIFO to clear IPC
>> interrupt.
>> + * Without purge IPC FIFO to 0 next IPC interrupts won't be generated.
>> + */
>> + while (ivpu_hw_reg_ipc_rx_count_get(vdev)) {
>
> Ick. Please no in the long term?
>
> This is an infinite loop. In hard IRQ context. Controlled by the device,
> which you probably shouldn't trust.
>
> However the real fix for this is to move to threaded_irqs. Which is going to
> be a huge refactor for you. Rate limiting doesn't appear viable.
>
> If I understand things correctly, the chances that the device will generate a
> large count, or update the count as fast or faster than the driver are low,
> but it should still be fixed.
>
> How about a high priority todo to convert to threaded irqs? At the same time
> you can update the return value for this function which seems to not be
> checked anywhere, and also the comment here which is not proper multi-line
> style.
OK, I've added this at the top of the TODO and fixed the comment.
Regarding the ivpu_ipc_irq_handler() return code it is checked in the next
patch in ivpu_wait_for_ready().
Regards,
Jacek
