Hi Laurent,
On 04/04/18 22:43, Laurent Pinchart wrote:
Hi Kieran,
On Wednesday, 4 April 2018 19:16:46 EEST Kieran Bingham wrote:
On 26/02/18 21:45, Laurent Pinchart wrote:
<snip>
-void vsp1_dl_list_commit(struct vsp1_dl_list *dl) +void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool notify)
Rather than changing the vsp1_dl_list_commit() function - would it be nicer to have an API to request or set the notify property? :
@@..@@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) ...
/* The BRx will be released, without sending an update to DRM */
if (drm_pipe->force_bru_release)
vsp1_dl_list_request_internal_notify(dl);vsp1_dl_list_commit(dl);
...
That's not a bad idea, but I wonder if it's worth it as we'll have to call an extra function for what is essentially an internal API. On the other hand this isn't a common case, so it's not a hot code path. We could also argue equally that it is the commit that is internal or that it is the display list that is
Aha, yes - it is more so that the commit is internal ...
for internal purpose. Do you think an extra function call is better ? If you do I'll change it.
so it could also instead be just a separate commit() function:
void vsp1_dl_list_commit_internal(struct vsp1_dl_list *dl) { dl->internal = true; vsp1_dl_list_commit(dl); }
...
{ /* The BRx will be released, without sending an update to DRM */ if (drm_pipe->force_bru_release) vsp1_dl_list_commit_internal(dl); else vsp1_dl_list_commit(dl); }
I'll leave the final implementation decision with you - I just thought that extending the commit call with a notify flag seemed odd.
Of course - that could also have been due to the naming of the 'notify' - if it was 'internal' as discussed in the other patches, then perhaps a flag on the function call is still a sensible way. It just affects the other commit usages, but there's only a total of three calls - so it's really not a big deal. If there were 12 call locations perhaps the function wrapper would have more merit - but probably not so much at 3 :D
-- Kieran