[PATCH xf86-video-amdgpu 04/10] Create drmmode_crtc_wait_pending_event helper macro

[Abramov, Slava]

From: Michel Dänzer <michel at daenzer.net>
Sent: Tuesday, August 22, 2017 1:52 AM
To: Abramov, Slava
Cc: amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH xf86-video-amdgpu 04/10] Create drmmode_crtc_wait_pending_event helper macro

On 18/08/17 11:51 PM, Abramov, Slava wrote:
> *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of
>
>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>> index 1a805b82d..bdd3866b8 100644
>> --- a/src/drmmode_display.c
>> +++ b/src/drmmode_display.c
>> @@ -96,6 +96,14 @@ AMDGPUZaphodStringMatches(ScrnInfoPtr pScrn, const
>> char *s, char *output_name)
>>          return FALSE;
>>  }
>>
>> +
>> +/* Wait for the boolean condition to be FALSE */
>> +#define drmmode_crtc_wait_pending_event(drmmode_crtc, fd, condition) \
>> +       do {} while ((condition) && \
>> +                    drmHandleEvent(fd,
>> &drmmode_crtc->drmmode->event_context) \
>> +                    > 0);
>> +
>> +
>
> [slava] The comment seems a little misleading to me.  The loop condition
> has actually two parts, and it's unclear what we're actually waiting
> for.  A side question: is it expected that the variable 'condition' will
> change while we're waiting?

Yes, that's the whole point. :) While (condition) evaluates to non-0, we
call drmHandleEvent(), which blocks until at least one DRM event
arrives, and triggers processing of arrived DRM events.

Maybe something like this would be better?

/* If condition evaluates to TRUE, process DRM events until condition
 * evaluates to FALSE.
 */

[slava] Yes, I like that :)


Anyway, this patch just ports the same change from xf86-video-ati, so
I'm not going to modify the comment in this patch. Do you want to send a
patch modifying the comment in either driver?

[slava] I will try to do this, as a military exercise, to get use to the process 😊


Slava
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170822/04b45c82/attachment.html>