Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Use the tiling from the modifier instead of the BO
On Tue, Dec 5, 2017 at 9:43 AM, Kristian Høgsberg wrote: > On Tue, Dec 5, 2017 at 8:49 AM, Jason Ekstrand wrote: >> On Tue, Dec 5, 2017 at 8:23 AM, Kristian Høgsberg >> wrote: >>> >>> On Tue, Dec 5, 2017 at 7:57 AM, Jason Ekstrand >>> wrote: >>> > On Tue, Dec 5, 2017 at 1:22 AM, Daniel Stone >>> > wrote: >>> >> >>> >> Hi, >>> >> >>> >> On 18 November 2017 at 00:10, Jason Ekstrand >>> >> wrote: >>> >> > This fixes a bug where we were taking the tiling from the BO >>> >> > regardless >>> >> > of what the modifier said. When we got images in from Vulkan where >>> >> > it >>> >> > doesn't set the tiling on the BO, we would treat them as linear even >>> >> > though the modifier expressly said to treat it as Y-tiled. >>> >> >>> >> For some reason I thought Ken had already reviewed this and it landed, >>> >> until Kristian mentioned last night. >>> > >>> > >>> > There's been some discussion about what the right thing to do is here. >>> > I've >>> > got a patch (which is now landed) which will make us set the tiling from >>> > Vulkan just like GL does. It's rather annoying but I think that's >>> > marginally better. >>> >>> I don't know that it's better - it reinforces the notion that you have >>> to make the kernel-side tiling match for the dma-buf import extension >>> to work. I think it'd be better to land these patches (btw, Rb: me >>> (can you even do parenthetical Rbs?)) >> >> >> I'll allow it. :) >> >>> >>> and call set-tiling in mesa. >> >> >> Yeah, I think that's reasonable. Really, this should only be a problem if >> we have a bunch of users trying to use the same BO with different modifiers. >> It can happen in theory (immagine two images in the same BO, one X and one >> Y) but it's a pretty crazy case. > > It's not a complete solution, of course, but at least we're kicking > the can down the road of increasingly esoteric use cases. > >>> >>> The >>> assumption being that if the tiling doesn't match the modifier, then >>> maybe the producer didn't care about kernel tiling? Alternatively, >>> could we set bo->tiling = INCONSISTENT or such in mesa and then use >>> that to avoid the gtt map paths? Actually, for compressed textures, you already must have a way to deal with glTexSubImage2D and similar without falling back to GTT maps. Can you just handle miptrees with mismatched modifiers (or perhaps just any valid modifier) the same way? Kristian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Use the tiling from the modifier instead of the BO
On Tue, Dec 5, 2017 at 8:49 AM, Jason Ekstrand wrote: > On Tue, Dec 5, 2017 at 8:23 AM, Kristian Høgsberg > wrote: >> >> On Tue, Dec 5, 2017 at 7:57 AM, Jason Ekstrand >> wrote: >> > On Tue, Dec 5, 2017 at 1:22 AM, Daniel Stone >> > wrote: >> >> >> >> Hi, >> >> >> >> On 18 November 2017 at 00:10, Jason Ekstrand >> >> wrote: >> >> > This fixes a bug where we were taking the tiling from the BO >> >> > regardless >> >> > of what the modifier said. When we got images in from Vulkan where >> >> > it >> >> > doesn't set the tiling on the BO, we would treat them as linear even >> >> > though the modifier expressly said to treat it as Y-tiled. >> >> >> >> For some reason I thought Ken had already reviewed this and it landed, >> >> until Kristian mentioned last night. >> > >> > >> > There's been some discussion about what the right thing to do is here. >> > I've >> > got a patch (which is now landed) which will make us set the tiling from >> > Vulkan just like GL does. It's rather annoying but I think that's >> > marginally better. >> >> I don't know that it's better - it reinforces the notion that you have >> to make the kernel-side tiling match for the dma-buf import extension >> to work. I think it'd be better to land these patches (btw, Rb: me >> (can you even do parenthetical Rbs?)) > > > I'll allow it. :) > >> >> and call set-tiling in mesa. > > > Yeah, I think that's reasonable. Really, this should only be a problem if > we have a bunch of users trying to use the same BO with different modifiers. > It can happen in theory (immagine two images in the same BO, one X and one > Y) but it's a pretty crazy case. It's not a complete solution, of course, but at least we're kicking the can down the road of increasingly esoteric use cases. >> >> The >> assumption being that if the tiling doesn't match the modifier, then >> maybe the producer didn't care about kernel tiling? Alternatively, >> could we set bo->tiling = INCONSISTENT or such in mesa and then use >> that to avoid the gtt map paths? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Use the tiling from the modifier instead of the BO
On Tue, Dec 5, 2017 at 8:23 AM, Kristian Høgsberg wrote: > On Tue, Dec 5, 2017 at 7:57 AM, Jason Ekstrand > wrote: > > On Tue, Dec 5, 2017 at 1:22 AM, Daniel Stone > wrote: > >> > >> Hi, > >> > >> On 18 November 2017 at 00:10, Jason Ekstrand > wrote: > >> > This fixes a bug where we were taking the tiling from the BO > regardless > >> > of what the modifier said. When we got images in from Vulkan where it > >> > doesn't set the tiling on the BO, we would treat them as linear even > >> > though the modifier expressly said to treat it as Y-tiled. > >> > >> For some reason I thought Ken had already reviewed this and it landed, > >> until Kristian mentioned last night. > > > > > > There's been some discussion about what the right thing to do is here. > I've > > got a patch (which is now landed) which will make us set the tiling from > > Vulkan just like GL does. It's rather annoying but I think that's > > marginally better. > > I don't know that it's better - it reinforces the notion that you have > to make the kernel-side tiling match for the dma-buf import extension > to work. I think it'd be better to land these patches (btw, Rb: me > (can you even do parenthetical Rbs?)) I'll allow it. :) > and call set-tiling in mesa. Yeah, I think that's reasonable. Really, this should only be a problem if we have a bunch of users trying to use the same BO with different modifiers. It can happen in theory (immagine two images in the same BO, one X and one Y) but it's a pretty crazy case. > The > assumption being that if the tiling doesn't match the modifier, then > maybe the producer didn't care about kernel tiling? Alternatively, > could we set bo->tiling = INCONSISTENT or such in mesa and then use > that to avoid the gtt map paths? > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Use the tiling from the modifier instead of the BO
On Tue, Dec 5, 2017 at 7:57 AM, Jason Ekstrand wrote: > On Tue, Dec 5, 2017 at 1:22 AM, Daniel Stone wrote: >> >> Hi, >> >> On 18 November 2017 at 00:10, Jason Ekstrand wrote: >> > This fixes a bug where we were taking the tiling from the BO regardless >> > of what the modifier said. When we got images in from Vulkan where it >> > doesn't set the tiling on the BO, we would treat them as linear even >> > though the modifier expressly said to treat it as Y-tiled. >> >> For some reason I thought Ken had already reviewed this and it landed, >> until Kristian mentioned last night. > > > There's been some discussion about what the right thing to do is here. I've > got a patch (which is now landed) which will make us set the tiling from > Vulkan just like GL does. It's rather annoying but I think that's > marginally better. I don't know that it's better - it reinforces the notion that you have to make the kernel-side tiling match for the dma-buf import extension to work. I think it'd be better to land these patches (btw, Rb: me (can you even do parenthetical Rbs?)) and call set-tiling in mesa. The assumption being that if the tiling doesn't match the modifier, then maybe the producer didn't care about kernel tiling? Alternatively, could we set bo->tiling = INCONSISTENT or such in mesa and then use that to avoid the gtt map paths? Kristian > > ___ > 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 2/2] i965/miptree: Use the tiling from the modifier instead of the BO
On Tue, Dec 5, 2017 at 1:22 AM, Daniel Stone wrote: > Hi, > > On 18 November 2017 at 00:10, Jason Ekstrand wrote: > > This fixes a bug where we were taking the tiling from the BO regardless > > of what the modifier said. When we got images in from Vulkan where it > > doesn't set the tiling on the BO, we would treat them as linear even > > though the modifier expressly said to treat it as Y-tiled. > > For some reason I thought Ken had already reviewed this and it landed, > until Kristian mentioned last night. There's been some discussion about what the right thing to do is here. I've got a patch (which is now landed) which will make us set the tiling from Vulkan just like GL does. It's rather annoying but I think that's marginally better. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/miptree: Use the tiling from the modifier instead of the BO
Hi, On 18 November 2017 at 00:10, Jason Ekstrand wrote: > This fixes a bug where we were taking the tiling from the BO regardless > of what the modifier said. When we got images in from Vulkan where it > doesn't set the tiling on the BO, we would treat them as linear even > though the modifier expressly said to treat it as Y-tiled. For some reason I thought Ken had already reviewed this and it landed, until Kristian mentioned last night. Oops. Series is: Reviewed-by: Daniel Stone Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] i965/miptree: Use the tiling from the modifier instead of the BO
This fixes a bug where we were taking the tiling from the BO regardless of what the modifier said. When we got images in from Vulkan where it doesn't set the tiling on the BO, we would treat them as linear even though the modifier expressly said to treat it as Y-tiled. Cc: mesa-sta...@lists.freedesktop.org --- src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c index 0155539..45a5d4f 100644 --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c @@ -986,7 +986,11 @@ intel_miptree_create_for_dri_image(struct brw_context *brw, uint32_t bo_tiling, bo_swizzle; brw_bo_get_tiling(image->bo, &bo_tiling, &bo_swizzle); - const enum isl_tiling tiling = isl_tiling_from_i915_tiling(bo_tiling); + const struct isl_drm_modifier_info *mod_info = + isl_drm_modifier_get_info(image->modifier); + + const enum isl_tiling tiling = + mod_info ? mod_info->tiling : isl_tiling_from_i915_tiling(bo_tiling); if (image->planar_format && image->planar_format->nplanes > 1) return miptree_create_for_planar_image(brw, image, target, tiling); @@ -1010,9 +1014,6 @@ intel_miptree_create_for_dri_image(struct brw_context *brw, if (!brw->ctx.TextureFormatSupported[format]) return NULL; - const struct isl_drm_modifier_info *mod_info = - isl_drm_modifier_get_info(image->modifier); - enum intel_miptree_create_flags mt_create_flags = 0; /* If this image comes in from a window system, we have different -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev