[PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps

[Eric Anholt]

Keith Packard <keithp at keithp.com> writes:

> GetScratchPixmapHeader should only be used for local memory pixmaps,
> as used by PutImage and friends. That's because when you free the
> scratch pixmap header, it doesn't actually free the pixmap; instead,
> it gets stuffed in pScreen->pScratchPixmap and any private data stored
> on it will be left hanging around forever.
>
> In the case of glamor, that private data includes all of the GL
> state. Using that scratch pixmap later results in glamor getting
> mightily confused as the pixmap and underlying objects do not match.
>
> Avoid this by allocating pixmap headers explicitly for this purpose.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
>  src/uxa/intel_display.c | 44 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
> index 0b83140..bcaafaa 100644
> --- a/src/uxa/intel_display.c
> +++ b/src/uxa/intel_display.c
> @@ -545,13 +545,31 @@ intel_crtc_shadow_allocate(xf86CrtcPtr crtc, int width, int height)
>  		return NULL;
>  	}
>  
> -	drm_intel_bo_disable_reuse(intel_crtc->rotate_bo);
> -
>  	intel_crtc->rotate_pitch = rotate_pitch;
>  	return intel_crtc->rotate_bo;
>  }

See comment below...

> @@ -602,8 +620,8 @@ intel_crtc_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr rotate_pixmap, void *data)
>  	struct intel_mode *mode = intel_crtc->mode;
>  
>  	if (rotate_pixmap) {
> -		intel_set_pixmap_bo(rotate_pixmap, NULL);
> -		FreeScratchPixmapHeader(rotate_pixmap);
> +                intel_set_pixmap_bo(rotate_pixmap, NULL);
> +                rotate_pixmap->drawable.pScreen->DestroyPixmap(rotate_pixmap);
>  	}
>  
>  	if (data) {
> @@ -1415,7 +1433,6 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>  	int	    i, old_width, old_height, old_pitch;
>  	int pitch;
>  	uint32_t tiling;
> -	ScreenPtr screen;
>  
>  	if (scrn->virtualX == width && scrn->virtualY == height)
>  		return TRUE;
> @@ -1430,8 +1447,7 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>  	old_front = intel->front_buffer;
>  
>  	if (intel->back_pixmap) {
> -		screen = intel->back_pixmap->drawable.pScreen;
> -		screen->DestroyPixmap(intel->back_pixmap);
> +		scrn->pScreen->DestroyPixmap(intel->back_pixmap);
>  		intel->back_pixmap = NULL;
>  	}
>  

Grumble, unrelated noise in the patch.

> @@ -1454,7 +1470,6 @@ intel_xf86crtc_resize(ScrnInfoPtr scrn, int width, int height)
>  	if (ret)
>  		goto fail;
>  
> -	drm_intel_bo_disable_reuse(intel->front_buffer);
>  	intel->front_pitch = pitch;
>  	intel->front_tiling = tiling;
>  

This change appears to be unrelated, and possibly harmful (if X has
dropped the last ref to the BO, but it's still the scanout buffer, a new
allocation would now reuse the BO and scribble on scanout until the next
modeset happens).

> @@ -2204,6 +2219,7 @@ Bool intel_crtc_on(xf86CrtcPtr crtc)
>  	return ret;
>  }
>  
> +
>  static PixmapPtr
>  intel_create_pixmap_for_bo(ScreenPtr pScreen, dri_bo *bo,
>  			   int width, int height,

Unrelated whitespace.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140730/95a60cac/attachment.sig>