[PATCH xf86-video-amdgpu] Simplify tracking of PRIME scanout pixmap

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Michel Dänzer
> Sent: Friday, May 12, 2017 6:11 AM
> To: amd-gfx at lists.freedesktop.org
> Subject: [PATCH xf86-video-amdgpu] Simplify tracking of PRIME scanout
> pixmap
> 
> From: Michel Dänzer <michel.daenzer at amd.com>
> 
> Remember the shared pixmap passed to drmmode_set_scanout_pixmap for
> each
> CRTC, and just compare against that.
> 
> Fixes leaving stale entries in ScreenRec::pixmap_dirty_list under some
> circumstances, which would usually result in use-after-free and a crash
> down the line.
> 
> (Ported from radeon commit 7dc68e26755466f9056f8c72195ab8690660693d)
> 
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>

Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  src/amdgpu_kms.c      |  7 ++-----
>  src/drmmode_display.c | 21 +++++++++++----------
>  src/drmmode_display.h |  3 +++
>  3 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
> index 4df81f993..a418cf9d3 100644
> --- a/src/amdgpu_kms.c
> +++ b/src/amdgpu_kms.c
> @@ -562,8 +562,7 @@ amdgpu_prime_dirty_to_crtc(PixmapDirtyUpdatePtr
> dirty)
>  		xf86CrtcPtr xf86_crtc = xf86_config->crtc[c];
>  		drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc-
> >driver_private;
> 
> -		if (drmmode_crtc->scanout[0].pixmap == dirty->slave_dst ||
> -			drmmode_crtc->scanout[1].pixmap == dirty-
> >slave_dst)
> +		if (drmmode_crtc->prime_scanout_pixmap == dirty->src)
>  			return xf86_crtc;
>  	}
> 
> @@ -576,13 +575,11 @@ amdgpu_prime_scanout_do_update(xf86CrtcPtr
> crtc, unsigned scanout_id)
>  	ScrnInfoPtr scrn = crtc->scrn;
>  	ScreenPtr screen = scrn->pScreen;
>  	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> -	PixmapPtr scanoutpix = crtc->randr_crtc->scanout_pixmap;
>  	PixmapDirtyUpdatePtr dirty;
>  	Bool ret = FALSE;
> 
>  	xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) {
> -		if (dirty->src == scanoutpix && dirty->slave_dst ==
> -		    drmmode_crtc->scanout[scanout_id ^ drmmode_crtc-
> >tear_free].pixmap) {
> +		if (dirty->src == drmmode_crtc->prime_scanout_pixmap) {
>  			RegionPtr region;
> 
>  			if (master_has_sync_shared_pixmap(scrn, dirty))
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index 9996d2f70..add8287a0 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -681,9 +681,7 @@ drmmode_crtc_prime_scanout_update(xf86CrtcPtr
> crtc, DisplayModePtr mode,
> 
>  		xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list,
>  					 ent) {
> -			if (dirty->src == crtc->randr_crtc->scanout_pixmap
> &&
> -			    dirty->slave_dst ==
> -			    drmmode_crtc->scanout[drmmode_crtc-
> >scanout_id].pixmap) {
> +			if (dirty->src == drmmode_crtc-
> >prime_scanout_pixmap) {
>  				dirty->slave_dst =
>  					drmmode_crtc-
> >scanout[scanout_id].pixmap;
>  				break;
> @@ -838,7 +836,7 @@ drmmode_set_mode_major(xf86CrtcPtr crtc,
> DisplayModePtr mode,
> 
>  		fb_id = drmmode->fb_id;
>  #ifdef AMDGPU_PIXMAP_SHARING
> -		if (crtc->randr_crtc && crtc->randr_crtc->scanout_pixmap) {
> +		if (drmmode_crtc->prime_scanout_pixmap) {
>  			drmmode_crtc_prime_scanout_update(crtc, mode,
> scanout_id,
>  							  &fb_id, &x, &y);
>  		} else
> @@ -1242,14 +1240,15 @@ static Bool
> drmmode_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix)
>  	PixmapDirtyUpdatePtr dirty;
> 
>  	xorg_list_for_each_entry(dirty, &screen->pixmap_dirty_list, ent) {
> -		if (dirty->slave_dst != drmmode_crtc-
> >scanout[scanout_id].pixmap)
> -			continue;
> -
> -		PixmapStopDirtyTracking(dirty->src, dirty->slave_dst);
> -		drmmode_crtc_scanout_free(drmmode_crtc);
> -		break;
> +		if (dirty->src == drmmode_crtc->prime_scanout_pixmap) {
> +			PixmapStopDirtyTracking(dirty->src, dirty-
> >slave_dst);
> +			break;
> +		}
>  	}
> 
> +	drmmode_crtc_scanout_free(drmmode_crtc);
> +	drmmode_crtc->prime_scanout_pixmap = NULL;
> +
>  	if (!ppix)
>  		return TRUE;
> 
> @@ -1266,6 +1265,8 @@ static Bool
> drmmode_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix)
>  		return FALSE;
>  	}
> 
> +	drmmode_crtc->prime_scanout_pixmap = ppix;
> +
>  #ifdef HAS_DIRTYTRACKING_ROTATION
>  	PixmapStartDirtyTracking(ppix, drmmode_crtc-
> >scanout[scanout_id].pixmap,
>  				 0, 0, 0, 0, RR_Rotate_0);
> diff --git a/src/drmmode_display.h b/src/drmmode_display.h
> index 2d5698f61..6a57fd23b 100644
> --- a/src/drmmode_display.h
> +++ b/src/drmmode_display.h
> @@ -83,6 +83,9 @@ typedef struct {
>  	unsigned scanout_id;
>  	Bool scanout_update_pending;
>  	Bool tear_free;
> +
> +	PixmapPtr prime_scanout_pixmap;
> +
>  	int dpms_mode;
>  	/* For when a flip is pending when DPMS off requested */
>  	int pending_dpms_mode;
> --
> 2.11.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx