Re: [Mesa-dev] [PATCH 00/13] Fix stencil texturing and BO caching bugs

2018-07-11 Thread Pohjolainen, Topi
On Fri, Jul 06, 2018 at 01:29:29PM -0700, Nanley Chery wrote:
> On Fri, Jul 06, 2018 at 03:36:01PM +0300, Pohjolainen, Topi wrote:
> > On Tue, Jun 12, 2018 at 12:21:52PM -0700, Nanley Chery wrote:
> > > This series fixes a couple stencil texturing bugs on HSW and
> > > cache-tracking for certain stencil BOs on all platforms.
> > > 
> > > Nanley Chery (13):
> > >   i965: Set the r8stencil flag in miptree_finish_write
> > >   i965/miptree: Set the r8stencil flag in map_depthstencil
> > >   i965/draw: Set the r8stencil flag after drawing
> > >   i965/draw: Fix adding the stencil bo to the depth cache
> > >   i965/miptree: Use make_surface in map_blit
> > >   i965/miptree: Delete MIPTREE_CREATE_LINEAR
> > >   i965/miptree: Share tiling_flags in miptree_create
> > >   i965/miptree: Share the miptree format in miptree_create
> > >   i965/miptree: Share alloc_flags in miptree_create
> > 
> > I'm not sure if this maintains the BO_ALLOC_BUSY.
> > 
> > >   i965/miptree: Add and use mt_surf_usage
> > >   i965/miptree: Refactor miptree_create
> > >   i965/miptree: Create the r8stencil_mt immediately
> > >   i965/miptree: Inline make_separate_stencil
> > 
> > Same here.
> 
> Yes, those two patches don't maintain BO_ALLOC_BUSY for the cases where
> the user creates a depth, depth-stencil, or stencil texture. It seems
> better this way though since, like a color texture, those are liable to
> be accessed with the CPU immediately.
> 
> I think we can maintain BO_ALLOC_BUSY for multisampled textures, since
> they will never be accessed on the CPU. I'll send a follow-up patch.
> Thoughts?

I'm fine with the change, just mention it in the commit message.

> 
> AFAICT, BO_ALLOC_BUSY wasn't used for those textures until commit
> a73d56dce37ae13f422215de1bf1fdfb8e2f6ed7, which allocated renderbuffers
> for depth stencil textures and implicitly (and perhaps accidentally)
> asked for a BUSY BO. I haven't thoroughly checked though.
> 
> > Otherwise the series is:
> > 
> > Reviewed-by: Topi Pohjolainen 
> > 
> 
> Thanks for the review!
> 
> -Nanley
> 
> > > 
> > >  src/mesa/drivers/dri/i965/brw_blorp.c |   6 +-
> > >  src/mesa/drivers/dri/i965/brw_clear.c |   8 -
> > >  src/mesa/drivers/dri/i965/brw_draw.c  |  14 +-
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 211 --
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   5 +-
> > >  src/mesa/drivers/dri/i965/intel_tex_image.c   |   3 -
> > >  6 files changed, 103 insertions(+), 144 deletions(-)
> > > 
> > > -- 
> > > 2.17.0
> > > 
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 00/13] Fix stencil texturing and BO caching bugs

2018-07-06 Thread Nanley Chery
On Fri, Jul 06, 2018 at 01:29:29PM -0700, Nanley Chery wrote:
> On Fri, Jul 06, 2018 at 03:36:01PM +0300, Pohjolainen, Topi wrote:
> > On Tue, Jun 12, 2018 at 12:21:52PM -0700, Nanley Chery wrote:
> > > This series fixes a couple stencil texturing bugs on HSW and
> > > cache-tracking for certain stencil BOs on all platforms.
> > > 
> > > Nanley Chery (13):
> > >   i965: Set the r8stencil flag in miptree_finish_write
> > >   i965/miptree: Set the r8stencil flag in map_depthstencil
> > >   i965/draw: Set the r8stencil flag after drawing
> > >   i965/draw: Fix adding the stencil bo to the depth cache
> > >   i965/miptree: Use make_surface in map_blit
> > >   i965/miptree: Delete MIPTREE_CREATE_LINEAR
> > >   i965/miptree: Share tiling_flags in miptree_create
> > >   i965/miptree: Share the miptree format in miptree_create
> > >   i965/miptree: Share alloc_flags in miptree_create
> > 
> > I'm not sure if this maintains the BO_ALLOC_BUSY.
> > 
> > >   i965/miptree: Add and use mt_surf_usage
> > >   i965/miptree: Refactor miptree_create
> > >   i965/miptree: Create the r8stencil_mt immediately
> > >   i965/miptree: Inline make_separate_stencil
> > 
> > Same here.
> 

I should mention that I attempted to answer your questions on those
patches here. Please let me know if I didn't.

-Nanley

> Yes, those two patches don't maintain BO_ALLOC_BUSY for the cases where
> the user creates a depth, depth-stencil, or stencil texture. It seems
> better this way though since, like a color texture, those are liable to
> be accessed with the CPU immediately.
> 
> I think we can maintain BO_ALLOC_BUSY for multisampled textures, since
> they will never be accessed on the CPU. I'll send a follow-up patch.
> Thoughts?
> 
> AFAICT, BO_ALLOC_BUSY wasn't used for those textures until commit
> a73d56dce37ae13f422215de1bf1fdfb8e2f6ed7, which allocated renderbuffers
> for depth stencil textures and implicitly (and perhaps accidentally)
> asked for a BUSY BO. I haven't thoroughly checked though.
> 
> > Otherwise the series is:
> > 
> > Reviewed-by: Topi Pohjolainen 
> > 
> 
> Thanks for the review!
> 
> -Nanley
> 
> > > 
> > >  src/mesa/drivers/dri/i965/brw_blorp.c |   6 +-
> > >  src/mesa/drivers/dri/i965/brw_clear.c |   8 -
> > >  src/mesa/drivers/dri/i965/brw_draw.c  |  14 +-
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 211 --
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   5 +-
> > >  src/mesa/drivers/dri/i965/intel_tex_image.c   |   3 -
> > >  6 files changed, 103 insertions(+), 144 deletions(-)
> > > 
> > > -- 
> > > 2.17.0
> > > 
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 00/13] Fix stencil texturing and BO caching bugs

2018-07-06 Thread Nanley Chery
On Fri, Jul 06, 2018 at 03:36:01PM +0300, Pohjolainen, Topi wrote:
> On Tue, Jun 12, 2018 at 12:21:52PM -0700, Nanley Chery wrote:
> > This series fixes a couple stencil texturing bugs on HSW and
> > cache-tracking for certain stencil BOs on all platforms.
> > 
> > Nanley Chery (13):
> >   i965: Set the r8stencil flag in miptree_finish_write
> >   i965/miptree: Set the r8stencil flag in map_depthstencil
> >   i965/draw: Set the r8stencil flag after drawing
> >   i965/draw: Fix adding the stencil bo to the depth cache
> >   i965/miptree: Use make_surface in map_blit
> >   i965/miptree: Delete MIPTREE_CREATE_LINEAR
> >   i965/miptree: Share tiling_flags in miptree_create
> >   i965/miptree: Share the miptree format in miptree_create
> >   i965/miptree: Share alloc_flags in miptree_create
> 
> I'm not sure if this maintains the BO_ALLOC_BUSY.
> 
> >   i965/miptree: Add and use mt_surf_usage
> >   i965/miptree: Refactor miptree_create
> >   i965/miptree: Create the r8stencil_mt immediately
> >   i965/miptree: Inline make_separate_stencil
> 
> Same here.

Yes, those two patches don't maintain BO_ALLOC_BUSY for the cases where
the user creates a depth, depth-stencil, or stencil texture. It seems
better this way though since, like a color texture, those are liable to
be accessed with the CPU immediately.

I think we can maintain BO_ALLOC_BUSY for multisampled textures, since
they will never be accessed on the CPU. I'll send a follow-up patch.
Thoughts?

AFAICT, BO_ALLOC_BUSY wasn't used for those textures until commit
a73d56dce37ae13f422215de1bf1fdfb8e2f6ed7, which allocated renderbuffers
for depth stencil textures and implicitly (and perhaps accidentally)
asked for a BUSY BO. I haven't thoroughly checked though.

> Otherwise the series is:
> 
> Reviewed-by: Topi Pohjolainen 
> 

Thanks for the review!

-Nanley

> > 
> >  src/mesa/drivers/dri/i965/brw_blorp.c |   6 +-
> >  src/mesa/drivers/dri/i965/brw_clear.c |   8 -
> >  src/mesa/drivers/dri/i965/brw_draw.c  |  14 +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 211 --
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   5 +-
> >  src/mesa/drivers/dri/i965/intel_tex_image.c   |   3 -
> >  6 files changed, 103 insertions(+), 144 deletions(-)
> > 
> > -- 
> > 2.17.0
> > 
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 00/13] Fix stencil texturing and BO caching bugs

2018-07-06 Thread Pohjolainen, Topi
On Tue, Jun 12, 2018 at 12:21:52PM -0700, Nanley Chery wrote:
> This series fixes a couple stencil texturing bugs on HSW and
> cache-tracking for certain stencil BOs on all platforms.
> 
> Nanley Chery (13):
>   i965: Set the r8stencil flag in miptree_finish_write
>   i965/miptree: Set the r8stencil flag in map_depthstencil
>   i965/draw: Set the r8stencil flag after drawing
>   i965/draw: Fix adding the stencil bo to the depth cache
>   i965/miptree: Use make_surface in map_blit
>   i965/miptree: Delete MIPTREE_CREATE_LINEAR
>   i965/miptree: Share tiling_flags in miptree_create
>   i965/miptree: Share the miptree format in miptree_create
>   i965/miptree: Share alloc_flags in miptree_create

I'm not sure if this maintains the BO_ALLOC_BUSY.

>   i965/miptree: Add and use mt_surf_usage
>   i965/miptree: Refactor miptree_create
>   i965/miptree: Create the r8stencil_mt immediately
>   i965/miptree: Inline make_separate_stencil

Same here. Otherwise the series is:

Reviewed-by: Topi Pohjolainen 

> 
>  src/mesa/drivers/dri/i965/brw_blorp.c |   6 +-
>  src/mesa/drivers/dri/i965/brw_clear.c |   8 -
>  src/mesa/drivers/dri/i965/brw_draw.c  |  14 +-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 211 --
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |   5 +-
>  src/mesa/drivers/dri/i965/intel_tex_image.c   |   3 -
>  6 files changed, 103 insertions(+), 144 deletions(-)
> 
> -- 
> 2.17.0
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev