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:


>>> -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;


        /* The BRx will be released, without sending an update to DRM */
        if (drm_pipe->force_bru_release)

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


Reply via email to