On Wed, Nov 19, 2014 at 05:47:11PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> It's supposed to contain all the information that is required for both
> kernel and user space vblank wait items, but not hold any information
> required by just one of them.
>
> For now, we just moved the struct members from one place to another,
> but the long term goal is that most of the drm.ko code will only
> handle "struct drm_vblank_wait_item", not knowing anything else. This
> will allow the callers to wrap this struct in their own private
> structures. This will happen in the next patches.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
I'd clarify the commit message here to say that it's *eventually* going
to contain all of the general data and be subclassed by callers. On
first read, I was confused here since you still have to use
e->event.user_data until patch #7.
Personally, I'd also just squash patch #4 into this one.
...
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index bc114f0..b8bc55a 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -665,10 +665,8 @@ typedef void (*drm_vblank_callback_t)(struct drm_device *dev,
> unsigned long seq, struct timeval *now,
> bool premature);
>
> -struct drm_pending_vblank_event {
> - struct drm_pending_event base;
> +struct drm_vblank_wait_item {
> int pipe;
> - struct drm_event_vblank event;
I know it's nitpicking, but the term 'item' seems a bit vague/ambiguous
to me. Maybe something like drm_vblank_task or drm_vblank_job would be
slightly more descriptive? In the same vein, I'd suggest something like
"drm_schedule_vblank_job()" in place of drm_wait_vblank_kernel() and
"drm_trigger_vblank_job()" in place of drm_wait_vblank_callback(). Of
course this is all pretty much personal opinion, so feel free to ignore
this suggestion if you disagree. :-)
Matt
>
> drm_vblank_callback_t callback;
> bool callback_from_work;
> @@ -681,6 +679,12 @@ struct drm_pending_vblank_event {
> } callback_args;
> };
>
> +struct drm_pending_vblank_event {
> + struct drm_pending_event base;
> + struct drm_event_vblank event;
> + struct drm_vblank_wait_item item;
> +};
> +
> struct drm_vblank_crtc {
> struct drm_device *dev; /* pointer to the drm_device */
> wait_queue_head_t queue; /**< VBLANK wait queue */
> --
> 2.1.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
