Re: xf86-video-intel: 3 commits - src/intel_display.c src/intel_dri.c src/intel.h

[Keith Packard <keithp@keithp.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2608 bytes --]

On Thu, 24 Mar 2011 02:02:56 +0100, Julien Cristau <jcristau@debian.org> wrote:
> Hi Keith,
> 
> a couple suggestions below from a quick look over these patches.

Thanks for your review!

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

Sadly, yes -- intel_vblank_handler gets stuffed into the
event_context.vblank_handler callback slot which uses 'void *' in the
function signature. As DRI2FrameEventPtr is a private type, we can't go
fix the public drmEventContext type to match.

A cleaner patch sequence would have removed the change from both
patches. Oops!

> > +	i830_dri2_add_frame_event(flip_info);
> > +
> 
> if (!i830_dri_add_frame_event(flip_info))
>     return FALSE;
> ?
...
> > +	i830_dri2_add_frame_event(swap_info);
> > +
> 
> if (!i830_dri2_add_frame_event(swap_info)
>     goto blit_fallback;
> ?

Good catch. Not quite that easy to fix; the swap_info needs to be freed,
and a partial add_frame_event must clean up after itself.

diff --git a/src/intel_dri.c b/src/intel_dri.c
index 3b80823..f7a4fc4 100644
--- a/src/intel_dri.c
+++ b/src/intel_dri.c
@@ -625,8 +625,10 @@ i830_dri2_add_frame_event(DRI2FrameEventPtr frame_event)
 	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))
+	if (!AddResource(frame_event->drawable_id, frame_event_drawable_type, frame_event)) {
+		FreeResourceByType(frame_event->client_id, frame_event_client_type, TRUE);
 		return FALSE;
+	}
 
 	return TRUE;
 }
@@ -705,7 +707,10 @@ 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_dri2_add_frame_event(flip_info)) {
+	    free(flip_info);
+	    return FALSE;
+	}
 
 	/* Page flip the full screen buffer */
 	back_priv = back->driverPrivate;
@@ -958,7 +963,10 @@ 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)) {
+	    free(swap_info);
+	    goto blit_fallback;
+	}
 
 	/* Get current count */
 	vbl.request.type = DRM_VBLANK_RELATIVE;

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx