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
