[Intel-gfx] xf86-video-intel: 3 commits - src/intel_display.c src/intel_dri.c src/intel.h

[Julien Cristau]

Hi Keith,

a couple suggestions below from a quick look over these patches.

On Wed, Mar 23, 2011 at 17:24:57 -0700, Keith Packard wrote:

> commit e1ff5182304e00c0d392092069422cae7626cf8d
> Author: Keith Packard <keithp at keithp.com>
> Date:   Wed Mar 9 17:00:41 2011 -0800
> 
>     Handle drawable/client destruction in pending swaps/flips
>     
>     A pending swap or flip holds references to drawables and clients which
>     become invalid when destroyed. Add suitable resources to the database
>     to track those lifetimes and clean up the pending data structure then.
>     
>     Later, when the pending swap or flip occurs, handle a missing drawable
>     by just discarding the flip or swap. Handle a missing client by not
>     sending an event or reply.
>     
>     Signed-off-by: Keith Packard <keithp at keithp.com>
> 
[...]
> diff --git a/src/intel_display.c b/src/intel_display.c
> index eb07cf5..4734844 100644
> --- a/src/intel_display.c
> +++ b/src/intel_display.c
> @@ -1512,7 +1512,7 @@ static const xf86CrtcConfigFuncsRec intel_xf86crtc_config_funcs = {
>  
>  static void
>  intel_vblank_handler(int fd, unsigned int frame, unsigned int tv_sec,
> -		       unsigned int tv_usec, DRI2FrameEventPtr event)
> +		       unsigned int tv_usec, void *event)

This seems to just revert a change from the previous commit?

>  {
>  	I830DRI2FrameEventHandler(frame, tv_sec, tv_usec, event);
>  }
> diff --git a/src/intel_dri.c b/src/intel_dri.c
> index 9e8c370..f039e9d 100644
> --- a/src/intel_dri.c
> +++ b/src/intel_dri.c
> @@ -577,6 +577,69 @@ I830DRI2DrawablePipe(DrawablePtr pDraw)
[...]
> +/*
> + * Hook this frame event into the server resource
> + * database so we can clean it up if the drawable or
> + * client exits while the swap is pending
> + */
> +static Bool
> +i830_dri2_add_frame_event(DRI2FrameEventPtr frame_event)
> +{
> +	frame_event->client_id = FakeClientID(frame_event->client->index);
> +
> +	if (!AddResource(frame_event->client_id, frame_event_client_type, frame_event))
> +		return FALSE;
> +
> +	if (!AddResource(frame_event->drawable_id, frame_event_drawable_type, frame_event))
> +		return FALSE;
> +
> +	return TRUE;
> +}
> +
> +static void
> +i830_dri2_del_frame_event(DRI2FrameEventPtr frame_event)
> +{
> +	if (frame_event->client_id)
> +		FreeResourceByType(frame_event->client_id, frame_event_client_type, FALSE);
> +	if (frame_event->drawable_id)
> +		FreeResourceByType(frame_event->drawable_id, frame_event_drawable_type, FALSE);
> +}
> +
>  static void
>  I830DRI2ExchangeBuffers(DrawablePtr draw, DRI2BufferPtr front,
>  			DRI2BufferPtr back)
> @@ -642,11 +705,18 @@ I830DRI2ScheduleFlip(struct intel_screen_private *intel,
>  	flip_info->event_data = data;
>  	flip_info->frame = target_msc;
>  
> +	i830_dri2_add_frame_event(flip_info);
> +

if (!i830_dri_add_frame_event(flip_info))
    return FALSE;
?

>  	/* Page flip the full screen buffer */
>  	back_priv = back->driverPrivate;
> -	return intel_do_pageflip(intel,
> -				 intel_get_pixmap_bo(back_priv->pixmap),
> -				 flip_info, ref_crtc_hw_id);
> +	if (intel_do_pageflip(intel,
> +			      intel_get_pixmap_bo(back_priv->pixmap),
> +			      flip_info, ref_crtc_hw_id))
> +		return TRUE;
> +
> +	i830_dri2_del_frame_event(flip_info);
> +	free(flip_info);
> +	return FALSE;
>  }
>  
>  static Bool
[...]
> @@ -876,6 +958,8 @@ I830DRI2ScheduleSwap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
>  	I830DRI2ReferenceBuffer(front);
>  	I830DRI2ReferenceBuffer(back);
>  
> +	i830_dri2_add_frame_event(swap_info);
> +

if (!i830_dri2_add_frame_event(swap_info)
    goto blit_fallback;
?

>  	/* Get current count */
>  	vbl.request.type = DRM_VBLANK_RELATIVE;
>  	if (pipe > 0)
[...]

Cheers,
Julien