[WESTON PATCH 2/2] compositor-drm: Watchdog timer implementation

Frederic Plourde frederic.plourde at collabora.co.uk
Thu Oct 30 10:16:49 PDT 2014


On 14-10-29 05:40 PM, Bill Spitzak wrote:
> On 10/29/2014 12:19 PM, Frederic Plourde wrote:
>
>> Mhh... you mean something like :
>>
>> static int
>> drm_output_watchdog_timer_create(struct drm_output *output)
>> {
>>      struct wl_event_loop *loop = NULL;
>>      struct weston_compositor *ec = output_base->compositor;
>>
>>      loop = wl_display_get_event_loop(ec->wl_display);
>>      if (loop) {
>>          output->wdt->timer = wl_event_loop_add_timer(loop,
>> drm_output_watchdog_bark,
>> ec->watchdog_timer_timeout);
>>      blah..blah...
>
> Yes that is what I meant.
>
>> If so, then I think I'd prefer to completely remove
>> 'drm_output_watchdog_timer_create" and inline it directly from
>> 'create_output_for_connector', since it's not a very big function anyway
>> and we're not sure as of today that we're going to reuse this for
>> anything else.
>
> Probably that is ok as well. The main problem is that it is impossible 
> to inline the "bark" function so one part of the code is in a 
> different place, which may mean it is clearer to put the reference to 
> that function up next to it too.
>
>> what do you think ?
>> I'm usually not a big fan of static functions with hard-coded values,
>> but if you tell me it's better for clarity... I might be tempted, dunno.
>
> Generally I don't think a generic function should be written until you 
> have at least 2 instances where it is used.

I think you won me over... Let's pretend we're not going to use that for 
anything else for now.
patch v2 coming up next :)

> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel



More information about the wayland-devel mailing list