Re: [PATCH v2 28/47] staging: vchi: Get rid of vchiq_shim's message callback

From: Jacopo Mondi
Date: Fri Aug 28 2020 - 10:27:25 EST


Hi Nicolas,

I'm working on a v2 of the bcm2835-isp support which was sent along
with UNICAM v4l2 driver and some misc changes you have collected in
this series. Reference to v1:
https://lore.kernel.org/linux-media/20200504092611.9798-1-laurent.pinchart@xxxxxxxxxxxxxxxx/

On Mon, Jun 29, 2020 at 05:09:26PM +0200, Nicolas Saenz Julienne wrote:
> As vchiq_shim's callback does nothing aside from pushing messages into
> the service's queue, let's bypass it and jump directly to the service's
> callbacks, letting them choose whether to use the message queue.

I admit this patch caused me some pain, as after a few days chasing
why the ISP got stuck in importing buffers into the VPU through the vc-sm-cma
driver I realized that this patch removed a significant part of the
process..

>
> It turns out most services don't need to use the message queue, which
> makes for simpler code in the end.
>
> -
> - if (reason == VCHIQ_MESSAGE_AVAILABLE)
> - vchiq_msg_queue_push(service->handle, header);

This one '-.-

I wonder if this was intentional and it is expected all services now
handle the message queue (it seems so according to your commit
message).

Fair enough, I could add in the vc-sma-cma callback a call to
vchiq_msg_queue_push() but I wonder if it wouldn't be better to do so
in vchiq_core.c:parse_rx_slots(), just before calling the service's
callback, so that this has not to be re-implemented in all services.

What would you suggest ?

And by the way I see mmal-vchiq.c:service_callback() releasing
messages but never pushing them to the queue. Is this intended as well ?

Thanks
j