Bug 104355 - Ivy Bridge ignores component mappings in texture views
Summary: Ivy Bridge ignores component mappings in texture views
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Vulkan/intel (show other bugs)
Version: 17.3
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Intel 3D Bugs Mailing List
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
: 108779 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-12-20 19:52 UTC by hrydgard
Modified: 2019-05-17 21:27 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Image clearly showing a red/blue swap (503.38 KB, image/png)
2017-12-20 19:52 UTC, hrydgard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description hrydgard 2017-12-20 19:52:59 UTC
Created attachment 136334 [details]
Image clearly showing a red/blue swap

The driver appears to swap the red and blue channels of VK_FORMAT_A1R5G5B5_UNORM_PACK16  textures. As can be seen in the game Tales of Eternia in my emulator PPSSPP:

https://user-images.githubusercontent.com/1274014/34218237-f0c1981a-e5ad-11e7-9b76-683a344c52fc.png

The game uses RGBA8888 textures for the background and ARGB1555 textures for the characters. They look a bit blue and ill, which is obviously an R/B channel swap. The colors are correct with the same code on other Vulkan implementations, so this points strongly to A1R5G5B5 being defined wrong in the driver.
Comment 1 Jason Ekstrand 2017-12-20 19:56:25 UTC
What hardware is this on?
Comment 2 hrydgard 2017-12-20 20:08:41 UTC
According to the reporting user, the hardware is "Gen7 (HD4000 mobile)". Driver version is 17.3.99.
Comment 3 hrydgard 2017-12-20 20:16:11 UTC
Should probably also mention that I am using a component mapping in the image View:

static const VkComponentMapping VULKAN_1555_SWIZZLE = { VK_COMPONENT_SWIZZLE_B, VK_COMPONENT_SWIZZLE_G, VK_COMPONENT_SWIZZLE_R, VK_COMPONENT_SWIZZLE_A };

This is assigned to VkImageViewCreateInfo.components when creating the image view used to read this texture.

This mapping does an R/B swap to make it match the PSP's channel order. So an alternative explanation is that this channel mapping isn't being applied correctly.
Comment 4 Jason Ekstrand 2017-12-20 20:38:54 UTC
Does the emulator use texture swizzling.  HD 4000 is Ivy Bridge which can't do texture swizzling.  (This is one of the reasons Ivy Bridge prints a big warning message when you try to use Vulkan on it.)
Comment 5 Jason Ekstrand 2017-12-20 21:15:15 UTC
I gave the bug a better title and we should leave it open.  Someone may get around to implementing support one of these days but it will require emitting shader code to do the swizzle.
Comment 6 hrydgard 2017-12-20 21:23:29 UTC
Oh, that's unfortunate.

Is there a robust way to detect GPUs that can't do this? If so, we could fall back to doing the color swap on the CPU beforehand instead of during swizzle.
Comment 7 Jason Ekstrand 2017-12-20 21:56:06 UTC
You can look at the deviceName and search for "Ivybridge" or "Bay Trail".
Comment 8 Ville Syrjala 2018-05-13 13:21:54 UTC
FYI I've taken a stab at implementing this:
git://github.com/vsyrjala/mesa.git anv_gen7_tex_swizzle

Fixed Talos Principle for me at least.

However Jason tells me there is a much better way of implementing this in the backend so I suppose this won't be the final version.
Comment 9 mrc_munir 2018-05-29 20:21:34 UTC
Thanks Ville Syrjala nice work, I confirm using your implemented branch anv_gen7_tex_swizzle is fixed in  PPSSPP too.

Regards,
mrc.
Comment 10 Jason Ekstrand 2019-01-14 19:24:32 UTC
*** Bug 108779 has been marked as a duplicate of this bug. ***
Comment 11 Ville Syrjala 2019-01-17 16:01:48 UTC
BTW I rebased the branch sometime late last year:
git://github.com/vsyrjala/mesa.git anv_gen7_tex_swizzle_3
Comment 12 Jason Ekstrand 2019-01-24 19:41:22 UTC
Ilia Iorin asked if I could provide a description of the "better" way to do this.  In principle, it's not that much different than what Ville did; it's just a bit more compact and uses a lot less instructions in the shader.

What I would recommend would be to push swizzle into the shader as a single 32-bit value (4 8-bit swizzle parameters) encoded as (R: 0, G: 4, B: 8, A: 12, ZERO: 16, ONE: 20).  Then add a special NIR intrinsic nir_intrinsic_swizzle_tex_val_intel which takes two sources: a vec4 and a uniform swizzle value which would be implemented in brw_fs_nir.cpp as something like this:

> const bool is_integer = nir_intrinsic_dst_type(intrin);
> dest.type = is_integer ? BRW_REGISTER_TYPE_UD : BRW_REGISTER_TYPE_F;
> fs_reg tmp = bld.vgrf(dest.type, 6);
> 
> for (unsigned i = 0; i < 4; i++)
>    bld.MOV(offset(tmp, bld, i), offset(src0, bld, i));
> 
> bld.MOV(offset(tmp, bld, 4), retype(brw_imm_ud(0), dest.type));
> if (is_integer)
>    bld.MOV(offset(tmp, bld, 5), brw_imm_ud(1));
> else
>    bld.MOV(offset(tmp, bld, 5), brw_imm_f(1));
> 
> bld.emit(SHADER_OPCODE_VEC4_TEX_SWIZZLE, dest, tmp, src1);

where SHADER_OPCODE_VEC4_TEX_SWIZZLE is a pseudo instruction that does the swizzle.  In particular, it would turn into something like this:

> add(4) r0<1>:uw src1<8,8,1>:ub 62:ud
> mov(8) g5<1>:ud r[a0.0]<1,0>:ud
> mov(8) g6<1>:ud r[a0.1]<1,0>:ud
> mov(8) g7<1>:ud r[a0.2]<1,0>:ud
> mov(8) g8<1>:ud r[a0.3]<1,0>:ud

The first add re-interprets the 32-bit swizzle value as 4 bytes and adds each to the byte offset of the register provided in src[0] of the VEC4_TEX_SWIZZLE instruction.  (The values we chose to represent our swizzles were conveniently in units of bytes so we get byte offsets).  The add then writes those into the first four address registers.  It's followed by four indirect MOV instructions to read from the specified component of src[0] and write into the destination.  This instruction would be very similar to the INDIRECT_MOV opcode we already have except that it does 4 MOVs at a time with a single address register set-up instruction.  Because we set up src[0] of the VEC4_TEX_SWIZZLE to be a vec6 which also contains the 0/1 constants, we can get all the different kinds of swizzles without doing a full matrix multiply or special-casing logic around the 0/1 constants.

These swizzling instructions will likely hurt texture performance due to stalling of the texture instructions caused by immediately adding a bunch of MOVs which use the instruction.  If we want to reduce that, we could potentially compile two versions of each shader: one which has swizzles and one which assumes all swizzles are RGBA and dynamically select between them at dispatch time based on a quick walk of the descriptor set to see if swizzles are needed.
Comment 13 Dominik Adrian Grzywak 2019-04-29 23:12:30 UTC
Could you guys commit this to the official Mesa?
Comment 14 Jason Ekstrand 2019-05-17 17:29:38 UTC
Done.  It's nothing like the patch from Ville because we recently completely rewrote the way you shove that sort of side-band data into the shader.  It's actually way easier now.

commit d2aa65eb1892f7b300ac24560f9dbda6b600b5a7 (public/master)
Author: Jason Ekstrand <jason@jlekstrand.net>
Date:   Fri May 17 10:04:58 2019 -0500

    anv: Emulate texture swizzle in the shader when needed
    
    Now that we have the descriptor buffer mechanism, emulated texture
    swizzle can be implemented in a very non-invasive way.  Previous
    attempts all tried to extend the push constant based image param
    mechanism which was gross.  This could, in theory, be done much faster
    with a magic back-end instruction which does indirect MOVs but Vulkan on
    IVB is already so slow this isn't going to matter much.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104355
    Cc: "19.1" <mesa-stable@lists.freedesktop.org>
    Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Comment 15 Dominik Adrian Grzywak 2019-05-17 21:27:01 UTC
Thanks


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.