On Thursday, August 13, 2015 02:57:20 PM Martin Peres wrote:
> On 07/08/15 23:13, Chris Wilson wrote:
> > intel_update_winsys_renderbuffer_miptree() will release the existing
> > miptree when wrapping a new DRI2 buffer, so we can remove the early
> > release and so prevent a NULL mt dereference should importing the new
> > DRI2 name fail for any reason. (Reusing the old DRI2 name will result
> > in the rendering going astray, to a stale buffer, and not shown on the
> > screen, but it allows us to issue a warning and not crash much later in
> > innocent code.)
> >
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > ---
> > src/mesa/drivers/dri/i965/brw_context.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> > b/src/mesa/drivers/dri/i965/brw_context.c
> > index e8d1396..72f3897 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > @@ -1388,7 +1388,6 @@ intel_process_dri2_buffer(struct brw_context *brw,
> > buffer->cpp, buffer->pitch);
> > }
> >
> > - intel_miptree_release(&rb->mt);
> > bo = drm_intel_bo_gem_create_from_name(brw->bufmgr, buffer_name,
> > buffer->name);
> > if (!bo) {
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86281
> Reviewed-by: Martin Peres <martin.pe...@linux.intel.com>
>
> If no-one has any objection, I will push the patch tomorrow to avoid the
> crashes experienced by a lot of users, myself included!This seems reasonable from a robustness point of view - try to limp along a bit further and hope the import works next time, somehow. That said, I do have one concern: we might get into trouble with multisample buffers. In the multisample case, irb->mt is the MSAA buffer, and irb->singlesample_mt is the actual single-sampled DRI2 buffer. Previously, this call always released irb->mt, deleting the MSAA buffer every time. Then, intel_update_winsys_renderbuffer_miptree would hit the !irb->mt case, creating a fresh new irb->mt, and setting irb->need_downsample = false. That may not happen now, which is a subtle change. (It might be OK...) If front buffer drawing, we call intel_renderbuffer_upsample(), which asserts that irb->need_downsample == false. So...if somehow we hit this path with a dirty multisample buffer and front buffer rendering...then I think we'd assert fail and die. I'm not sure if that can happen; it at least seems unlikely... Do you think the new multisampled behavior will work OK? If so, perhaps we can at least leave a note about it in the commit message. Thanks! --Ken
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
