Re: [Intel-gfx] [PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps
Keith Packard kei...@keithp.com writes: Eric Anholt e...@anholt.net writes: 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). Yeah, it's unrelated. intel_allocate_framebuffer calls disable_reuse, so there's no need to call it from these two other places. I'll split that change out into a separate patch with separate comment. Unrelated whitespace. There are a bunch of whitespace fixups; should I pull those into a separate patch or just leave them scattered in the first patch to change a file? One patch at the front is fine with me. pgpytHLVSAeTJ.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps
Keith Packard kei...@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 kei...@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.
pgpBxJNfF4O_i.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps
Eric Anholt e...@anholt.net writes: 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). Yeah, it's unrelated. intel_allocate_framebuffer calls disable_reuse, so there's no need to call it from these two other places. I'll split that change out into a separate patch with separate comment. Unrelated whitespace. There are a bunch of whitespace fixups; should I pull those into a separate patch or just leave them scattered in the first patch to change a file? -- keith.pack...@intel.com pgpius4ilGE8T.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/12] Don't use GetScratchPixmapHeader for shadow pixmaps
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 kei...@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;
}
static PixmapPtr
+intel_create_pixmap_header(ScreenPtr pScreen, int width, int height, int depth,
+ int bitsPerPixel, int devKind, void *pPixData)
+{
+PixmapPtr pixmap;
+
+/* width and height of 0 means don't allocate any pixmap data */
+pixmap = (*pScreen-CreatePixmap) (pScreen, 0, 0, depth, 0);
+
+if (pixmap) {
+if ((*pScreen-ModifyPixmapHeader) (pixmap, width, height,
depth,
+bitsPerPixel, devKind,
pPixData))
+{
+return pixmap;
+}
+(*pScreen-DestroyPixmap) (pixmap);
+}
+return NullPixmap;
+}
+
+static PixmapPtr
intel_crtc_shadow_create(xf86CrtcPtr crtc, void *data, int width, int height)
{
ScrnInfoPtr scrn = crtc-scrn;
@@ -573,12 +591,12 @@ intel_crtc_shadow_create(xf86CrtcPtr crtc, void *data,
int width, int height)
return NULL;
}
- rotate_pixmap = GetScratchPixmapHeader(scrn-pScreen,
- width, height,
- scrn-depth,
- scrn-bitsPerPixel,
- intel_crtc-rotate_pitch,
- NULL);
+ rotate_pixmap = intel_create_pixmap_header(scrn-pScreen,
+ width, height,
+ scrn-depth,
+ scrn-bitsPerPixel,
+ intel_crtc-rotate_pitch,
+ NULL);
if (rotate_pixmap == NULL) {
xf86DrvMsg(scrn-scrnIndex, X_ERROR,
@@ -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;
}
@@ -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;
@@ -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,
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
