* Re: [PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer format during page flip
2020-12-29 21:10 [PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer format during page flip Zhan Liu
@ 2021-01-03 15:43 ` Christian König
2021-01-07 17:32 ` Daniel Vetter
2021-01-05 12:51 ` Dan Carpenter
1 sibling, 1 reply; 5+ messages in thread
From: Christian König @ 2021-01-03 15:43 UTC (permalink / raw)
To: Zhan Liu, amd-gfx, Nikola.Cornij, Stylon.Wang, Chao-kai.Wang,
dri-devel, Nicholas.Kazlauskas, bas
Am 29.12.20 um 22:10 schrieb Zhan Liu:
> [Why]
> Driver cannot change amdgpu framebuffer (afb) format while doing
> page flip. Force system doing so will cause ioctl error, and result in
> breaking several functionalities including FreeSync.
>
> If afb format is forced to change during page flip, following message
> will appear in dmesg.log:
>
> "[drm:drm_mode_page_flip_ioctl [drm]]
> Page flip is not allowed to change frame buffer format."
>
> [How]
> Do not change afb format while doing page flip. It is okay to check
> whether afb format is valid here, however, forcing afb format change
> shouldn't happen here.
I don't think this we can do this.
It is perfectly valid for a page flip to switch between linear and tiled
formats on an I+A or A+A laptop.
Christian.
>
> Signed-off-by: Zhan Liu <zhan.liu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Thanks Nick and Bas. Here is my second patch for review.
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index a638709e9c92..8a12e27ff4ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -832,7 +832,8 @@ static int convert_tiling_flags_to_modifier(struct amdgpu_framebuffer *afb)
> if (!format_info)
> return -EINVAL;
>
> - afb->base.format = format_info;
> + if (!afb->base.format)
> + afb->base.format = format_info;
> }
> }
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer format during page flip
2020-12-29 21:10 [PATCH v2] drm/amdgpu: Do not change amdgpu framebuffer format during page flip Zhan Liu
2021-01-03 15:43 ` Christian König
@ 2021-01-05 12:51 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-01-05 12:51 UTC (permalink / raw)
To: kbuild, Zhan Liu, amd-gfx, Nikola.Cornij, Stylon.Wang,
Chao-kai.Wang, dri-devel, Nicholas.Kazlauskas, bas
Cc: kbuild-all, lkp
[-- Attachment #1: Type: text/plain, Size: 14891 bytes --]
Hi Zhan,
url: https://github.com/0day-ci/linux/commits/Zhan-Liu/drm-amdgpu-Do-not-change-amdgpu-framebuffer-format-during-page-flip/20201230-051134
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dea8dcf2a9fa8cc540136a6cd885c3beece16ec3
config: x86_64-randconfig-m031-20201229 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c:831 convert_tiling_flags_to_modifier() warn: variable dereferenced before check 'afb->base.format' (see line 826)
vim +831 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 678 static int convert_tiling_flags_to_modifier(struct amdgpu_framebuffer *afb)
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 679 {
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 680 struct amdgpu_device *adev = drm_to_adev(afb->base.dev);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 681 uint64_t modifier = 0;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 682
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 683 if (!afb->tiling_flags || !AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE)) {
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 684 modifier = DRM_FORMAT_MOD_LINEAR;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 685 } else {
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 686 int swizzle = AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 687 bool has_xor = swizzle >= 16;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 688 int block_size_bits;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 689 int version;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 690 int pipe_xor_bits = 0;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 691 int bank_xor_bits = 0;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 692 int packers = 0;
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 693 int rb = 0;
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 694 int pipes = ilog2(adev->gfx.config.gb_addr_config_fields.num_pipes);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 695 uint32_t dcc_offset = AMDGPU_TILING_GET(afb->tiling_flags, DCC_OFFSET_256B);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 696
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 697 switch (swizzle >> 2) {
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 698 case 0: /* 256B */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 699 block_size_bits = 8;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 700 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 701 case 1: /* 4KiB */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 702 case 5: /* 4KiB _X */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 703 block_size_bits = 12;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 704 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 705 case 2: /* 64KiB */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 706 case 4: /* 64 KiB _T */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 707 case 6: /* 64 KiB _X */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 708 block_size_bits = 16;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 709 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 710 default:
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 711 /* RESERVED or VAR */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 712 return -EINVAL;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 713 }
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 714
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 715 if (adev->asic_type >= CHIP_SIENNA_CICHLID)
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 716 version = AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 717 else if (adev->family == AMDGPU_FAMILY_NV)
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 718 version = AMD_FMT_MOD_TILE_VER_GFX10;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 719 else
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 720 version = AMD_FMT_MOD_TILE_VER_GFX9;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 721
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 722 switch (swizzle & 3) {
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 723 case 0: /* Z microtiling */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 724 return -EINVAL;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 725 case 1: /* S microtiling */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 726 if (!has_xor)
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 727 version = AMD_FMT_MOD_TILE_VER_GFX9;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 728 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 729 case 2:
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 730 if (!has_xor && afb->base.format->cpp[0] != 4)
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 731 version = AMD_FMT_MOD_TILE_VER_GFX9;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 732 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 733 case 3:
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 734 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 735 }
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 736
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 737 if (has_xor) {
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 738 switch (version) {
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 739 case AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS:
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 740 pipe_xor_bits = min(block_size_bits - 8, pipes);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 741 packers = min(block_size_bits - 8 - pipe_xor_bits,
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 742 ilog2(adev->gfx.config.gb_addr_config_fields.num_pkrs));
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 743 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 744 case AMD_FMT_MOD_TILE_VER_GFX10:
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 745 pipe_xor_bits = min(block_size_bits - 8, pipes);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 746 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 747 case AMD_FMT_MOD_TILE_VER_GFX9:
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 748 rb = ilog2(adev->gfx.config.gb_addr_config_fields.num_se) +
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 749 ilog2(adev->gfx.config.gb_addr_config_fields.num_rb_per_se);
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 750 pipe_xor_bits = min(block_size_bits - 8, pipes +
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 751 ilog2(adev->gfx.config.gb_addr_config_fields.num_se));
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 752 bank_xor_bits = min(block_size_bits - 8 - pipe_xor_bits,
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 753 ilog2(adev->gfx.config.gb_addr_config_fields.num_banks));
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 754 break;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 755 }
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 756 }
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 757
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 758 modifier = AMD_FMT_MOD |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 759 AMD_FMT_MOD_SET(TILE, AMDGPU_TILING_GET(afb->tiling_flags, SWIZZLE_MODE)) |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 760 AMD_FMT_MOD_SET(TILE_VERSION, version) |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 761 AMD_FMT_MOD_SET(PIPE_XOR_BITS, pipe_xor_bits) |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 762 AMD_FMT_MOD_SET(BANK_XOR_BITS, bank_xor_bits) |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 763 AMD_FMT_MOD_SET(PACKERS, packers);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 764
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 765 if (dcc_offset != 0) {
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 766 bool dcc_i64b = AMDGPU_TILING_GET(afb->tiling_flags, DCC_INDEPENDENT_64B) != 0;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 767 bool dcc_i128b = version >= AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS;
816853f9dc4057b6 Bas Nieuwenhuizen 2020-11-11 768 const struct drm_format_info *format_info;
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 769 u64 render_dcc_offset;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 770
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 771 /* Enable constant encode on RAVEN2 and later. */
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 772 bool dcc_constant_encode = adev->asic_type > CHIP_RAVEN ||
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 773 (adev->asic_type == CHIP_RAVEN &&
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 774 adev->external_rev_id >= 0x81);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 775
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 776 int max_cblock_size = dcc_i64b ? AMD_FMT_MOD_DCC_BLOCK_64B :
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 777 dcc_i128b ? AMD_FMT_MOD_DCC_BLOCK_128B :
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 778 AMD_FMT_MOD_DCC_BLOCK_256B;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 779
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 780 modifier |= AMD_FMT_MOD_SET(DCC, 1) |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 781 AMD_FMT_MOD_SET(DCC_CONSTANT_ENCODE, dcc_constant_encode) |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 782 AMD_FMT_MOD_SET(DCC_INDEPENDENT_64B, dcc_i64b) |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 783 AMD_FMT_MOD_SET(DCC_INDEPENDENT_128B, dcc_i128b) |
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 784 AMD_FMT_MOD_SET(DCC_MAX_COMPRESSED_BLOCK, max_cblock_size);
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 785
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 786 afb->base.offsets[1] = dcc_offset * 256 + afb->base.offsets[0];
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 787 afb->base.pitches[1] =
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 788 AMDGPU_TILING_GET(afb->tiling_flags, DCC_PITCH_MAX) + 1;
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 789
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 790 /*
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 791 * If the userspace driver uses retiling the tiling flags do not contain
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 792 * info on the renderable DCC buffer. Luckily the opaque metadata contains
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 793 * the info so we can try to extract it. The kernel does not use this info
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 794 * but we should convert it to a modifier plane for getfb2, so the
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 795 * userspace driver that gets it doesn't have to juggle around another DCC
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 796 * plane internally.
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 797 */
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 798 if (extract_render_dcc_offset(adev, afb->base.obj[0],
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 799 &render_dcc_offset) == 0 &&
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 800 render_dcc_offset != 0 &&
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 801 render_dcc_offset != afb->base.offsets[1] &&
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 802 render_dcc_offset < UINT_MAX) {
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 803 uint32_t dcc_block_bits; /* of base surface data */
816853f9dc4057b6 Bas Nieuwenhuizen 2020-11-11 804
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 805 modifier |= AMD_FMT_MOD_SET(DCC_RETILE, 1);
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 806 afb->base.offsets[2] = render_dcc_offset;
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 807
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 808 if (adev->family >= AMDGPU_FAMILY_NV) {
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 809 int extra_pipe = 0;
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 810
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 811 if (adev->asic_type >= CHIP_SIENNA_CICHLID &&
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 812 pipes == packers && pipes > 1)
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 813 extra_pipe = 1;
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 814
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 815 dcc_block_bits = max(20, 16 + pipes + extra_pipe);
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 816 } else {
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 817 modifier |= AMD_FMT_MOD_SET(RB, rb) |
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 818 AMD_FMT_MOD_SET(PIPE, pipes);
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 819 dcc_block_bits = max(20, 18 + rb);
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 820 }
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 821
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 822 dcc_block_bits -= ilog2(afb->base.format->cpp[0]);
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 823 afb->base.pitches[2] = ALIGN(afb->base.width,
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 824 1u << ((dcc_block_bits + 1) / 2));
1331e6304f5d924a Bas Nieuwenhuizen 2020-11-11 825 }
816853f9dc4057b6 Bas Nieuwenhuizen 2020-11-11 @826 format_info = amdgpu_lookup_format_info(afb->base.format->format,
^^^^^^^^^^^^^^^^^^
Dereferenced here
816853f9dc4057b6 Bas Nieuwenhuizen 2020-11-11 827 modifier);
816853f9dc4057b6 Bas Nieuwenhuizen 2020-11-11 828 if (!format_info)
816853f9dc4057b6 Bas Nieuwenhuizen 2020-11-11 829 return -EINVAL;
816853f9dc4057b6 Bas Nieuwenhuizen 2020-11-11 830
0a6703fd10d106b3 Zhan Liu 2020-12-29 @831 if (!afb->base.format)
^^^^^^^^^^^^^^^^^
New NULL check is too late.
816853f9dc4057b6 Bas Nieuwenhuizen 2020-11-11 832 afb->base.format = format_info;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 833 }
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 834 }
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 835
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 836 afb->base.modifier = modifier;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 837 afb->base.flags |= DRM_MODE_FB_MODIFIERS;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 838 return 0;
08d769151dc9690a Bas Nieuwenhuizen 2020-09-02 839 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34870 bytes --]
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread