On Sep 22, 2016 11:11 PM, "Nanley Chery" <<a href="mailto:nanleychery@gmail.com">nanleychery@gmail.com</a>> wrote: > > On Wed, Sep 14, 2016 at 01:28:23PM -0700, Jason Ekstrand wrote: > > Compressed 1-D textures are a well-defined thing in both GL and Vulkan. > > Could you provide a reference? I could not find any documentation on 1D > compressed textures. Instead I found that OpenGL disallows them and > Vulkan does not explicitly mention them. In OpenGL CompressedTexImage1D > returns INVALID_ENUM for ASTC, LATC, S3TC, RGTC, BPTC, and FXT1. The same > return value is implied for ASTC. In Vulkan, drivers are required to > support one of the following: BPTC for 2D and 3D images, ASTC LDR for 2D > images, or ETC2 and EAC for 2D images. OK, so they don't normally exist... > Our HW also doesn't support this. RENDER_SURFACE_STATE says: > * This field cannot be a compressed (BC*, DXT*, FXT*, ETC*, EAC*) format > if the Surface Type is SURFTYPE_1D. > * This field cannot be ASTC format if the Surface Type is SURFTYPE_1D. Right. Maybe assert in surface state setup about that. > I retract my earlier review feedback - the isl_*_gen9_1d functions > should not be updated because ISL_DIM_LAYOUT_GEN9_1D compressed > textures cannot exist. > > > > > v2: Fix some asserts (Nanley) > > > > Signed-off-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > > --- > > src/intel/isl/isl.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c > > index a75fddf..710c990 100644 > > --- a/src/intel/isl/isl.c > > +++ b/src/intel/isl/isl.c > > @@ -518,7 +518,6 @@ isl_calc_phys_level0_extent_sa(const struct isl_device *dev, > > assert(info->height == 1); > > assert(info->depth == 1); > > assert(info->samples == 1); > > - assert(!isl_format_is_compressed(info->format)); > > If you'd like to make this change for HiZ, I think we should keep this > assert and OR in a requirement that the format or tiling is HiZ. Eh... We also need it for CCS (both gen9 compression and fast clears) and I think the calculations are fairly well-defined. I don't see why we shouldn't just allow it in the calculation code. It wouldn't hurt to have an assert somewhere that prevents creating "normal" compressed 1D surfaces. As I said above, maybe assert in the surface state setup code or something. --Jason > -Nanley > > > > > switch (dim_layout) { > > case ISL_DIM_LAYOUT_GEN4_3D: > > @@ -527,8 +526,8 @@ isl_calc_phys_level0_extent_sa(const struct isl_device *dev, > > case ISL_DIM_LAYOUT_GEN9_1D: > > case ISL_DIM_LAYOUT_GEN4_2D: > > *phys_level0_sa = (struct isl_extent4d) { > > - .w = info->width, > > - .h = 1, > > + .w = isl_align_npot(info->width, fmtl->bw), > > + .h = fmtl->bh, > > .d = 1, > > .a = info->array_len, > > }; > > @@ -757,7 +756,7 @@ isl_calc_phys_slice0_extent_sa_gen9_1d( > > { > > MAYBE_UNUSED const struct isl_format_layout *fmtl = isl_format_get_layout(info->format); > > > > - assert(phys_level0_sa->height == 1); > > + assert(phys_level0_sa->height == fmtl->bh); > > assert(phys_level0_sa->depth == 1); > > assert(info->samples == 1); > > assert(image_align_sa->w >= fmtl->bw); > > @@ -1567,9 +1566,12 @@ get_image_offset_sa_gen9_1d(const struct isl_surf *surf, > > uint32_t *x_offset_sa, > > uint32_t *y_offset_sa) > > { > > + MAYBE_UNUSED const struct isl_format_layout *fmtl = > > + isl_format_get_layout(surf->format); > > + > > assert(level < surf->levels); > > assert(layer < surf->phys_level0_sa.array_len); > > - assert(surf->phys_level0_sa.height == 1); > > + assert(surf->phys_level0_sa.height == fmtl->bh); > > assert(surf->phys_level0_sa.depth == 1); > > assert(surf->samples == 1); > > > > -- > > 2.5.0.400.gff86faf > > > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>