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

Eric Anholt
2014-07-31T01:33:07Z

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

Keith Packard <keithp-aN4HjG94KOLQT0dZR+AlfA@public.gmane.org> 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-aN4HjG94KOLQT0dZR+AlfA@public.gmane.org>
> ---
> 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.

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

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

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel