Hi Maxime, I love your patch! Perhaps something to improve: [auto build test WARNING on drm-tip/drm-tip] [also build test WARNING on linus/master v5.10-rc6 next-20201204] [cannot apply to drm-intel/for-linux-next anholt/for-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Maxime-Ripard/vc4-Convert-to-drm_atomic_helper_commit/20201204-231528 base: git://anongit.freedesktop.org/drm/drm-tip drm-tip config: riscv-randconfig-r014-20201204 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 32c501dd88b62787d3a5ffda7aabcf4650dbe3cd) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/0day-ci/linux/commit/1ac52fbc9e5e40633ecb194184b4ba69937e8b55 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Maxime-Ripard/vc4-Convert-to-drm_atomic_helper_commit/20201204-231528 git checkout 1ac52fbc9e5e40633ecb194184b4ba69937e8b55 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@xxxxxxxxx> All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/vc4/vc4_kms.c:16: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:31: In file included from include/linux/fb.h:17: In file included from arch/riscv/include/asm/io.h:149: include/asm-generic/io.h:556:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] return inb(addr); ^~~~~~~~~ arch/riscv/include/asm/io.h:55:76: note: expanded from macro 'inb' #define inb(c) ({ u8 __v; __io_pbr(); __v = readb_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; }) ~~~~~~~~~~ ^ arch/riscv/include/asm/mmio.h:87:48: note: expanded from macro 'readb_cpu' #define readb_cpu(c) ({ u8 __r = __raw_readb(c); __r; }) ^ In file included from drivers/gpu/drm/vc4/vc4_kms.c:16: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:31: In file included from include/linux/fb.h:17: In file included from arch/riscv/include/asm/io.h:149: include/asm-generic/io.h:564:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] return inw(addr); ^~~~~~~~~ arch/riscv/include/asm/io.h:56:76: note: expanded from macro 'inw' #define inw(c) ({ u16 __v; __io_pbr(); __v = readw_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; }) ~~~~~~~~~~ ^ arch/riscv/include/asm/mmio.h:88:76: note: expanded from macro 'readw_cpu' #define readw_cpu(c) ({ u16 __r = le16_to_cpu((__force __le16)__raw_readw(c)); __r; }) ^ include/uapi/linux/byteorder/little_endian.h:36:51: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) ((__force __u16)(__le16)(x)) ^ In file included from drivers/gpu/drm/vc4/vc4_kms.c:16: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:31: In file included from include/linux/fb.h:17: In file included from arch/riscv/include/asm/io.h:149: include/asm-generic/io.h:572:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] return inl(addr); ^~~~~~~~~ arch/riscv/include/asm/io.h:57:76: note: expanded from macro 'inl' #define inl(c) ({ u32 __v; __io_pbr(); __v = readl_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; }) ~~~~~~~~~~ ^ arch/riscv/include/asm/mmio.h:89:76: note: expanded from macro 'readl_cpu' #define readl_cpu(c) ({ u32 __r = le32_to_cpu((__force __le32)__raw_readl(c)); __r; }) ^ include/uapi/linux/byteorder/little_endian.h:34:51: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) ^ In file included from drivers/gpu/drm/vc4/vc4_kms.c:16: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:31: In file included from include/linux/fb.h:17: In file included from arch/riscv/include/asm/io.h:149: include/asm-generic/io.h:580:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] outb(value, addr); ^~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:59:68: note: expanded from macro 'outb' #define outb(v,c) ({ __io_pbw(); writeb_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); }) ~~~~~~~~~~ ^ arch/riscv/include/asm/mmio.h:91:52: note: expanded from macro 'writeb_cpu' #define writeb_cpu(v, c) ((void)__raw_writeb((v), (c))) ^ In file included from drivers/gpu/drm/vc4/vc4_kms.c:16: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:31: In file included from include/linux/fb.h:17: In file included from arch/riscv/include/asm/io.h:149: include/asm-generic/io.h:588:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] outw(value, addr); ^~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:60:68: note: expanded from macro 'outw' #define outw(v,c) ({ __io_pbw(); writew_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); }) ~~~~~~~~~~ ^ arch/riscv/include/asm/mmio.h:92:76: note: expanded from macro 'writew_cpu' #define writew_cpu(v, c) ((void)__raw_writew((__force u16)cpu_to_le16(v), (c))) ^ In file included from drivers/gpu/drm/vc4/vc4_kms.c:16: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:31: In file included from include/linux/fb.h:17: In file included from arch/riscv/include/asm/io.h:149: include/asm-generic/io.h:596:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] outl(value, addr); ^~~~~~~~~~~~~~~~~ arch/riscv/include/asm/io.h:61:68: note: expanded from macro 'outl' #define outl(v,c) ({ __io_pbw(); writel_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); }) ~~~~~~~~~~ ^ arch/riscv/include/asm/mmio.h:93:76: note: expanded from macro 'writel_cpu' #define writel_cpu(v, c) ((void)__raw_writel((__force u32)cpu_to_le32(v), (c))) ^ In file included from drivers/gpu/drm/vc4/vc4_kms.c:16: In file included from include/drm/drm_atomic.h:31: In file included from include/drm/drm_crtc.h:31: In file included from include/linux/fb.h:17: In file included from arch/riscv/include/asm/io.h:149: include/asm-generic/io.h:1005:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port; ~~~~~~~~~~ ^ >> drivers/gpu/drm/vc4/vc4_kms.c:902:4: warning: variable 'unassigned_channels' is uninitialized when used here [-Wuninitialized] unassigned_channels |= BIT(i); ^~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/vc4/vc4_kms.c:893:34: note: initialize the variable 'unassigned_channels' to silence this warning unsigned int unassigned_channels; ^ = 0 8 warnings generated. vim +/unassigned_channels +902 drivers/gpu/drm/vc4/vc4_kms.c 856 857 /* 858 * The BCM2711 HVS has up to 7 outputs connected to the pixelvalves and 859 * the TXP (and therefore all the CRTCs found on that platform). 860 * 861 * The naive (and our initial) implementation would just iterate over 862 * all the active CRTCs, try to find a suitable FIFO, and then remove it 863 * from the pool of available FIFOs. However, there are a few corner 864 * cases that need to be considered: 865 * 866 * - When running in a dual-display setup (so with two CRTCs involved), 867 * we can update the state of a single CRTC (for example by changing 868 * its mode using xrandr under X11) without affecting the other. In 869 * this case, the other CRTC wouldn't be in the state at all, so we 870 * need to consider all the running CRTCs in the DRM device to assign 871 * a FIFO, not just the one in the state. 872 * 873 * - To fix the above, we can't use drm_atomic_get_crtc_state on all 874 * enabled CRTCs to pull their CRTC state into the global state, since 875 * a page flip would start considering their vblank to complete. Since 876 * we don't have a guarantee that they are actually active, that 877 * vblank might never happen, and shouldn't even be considered if we 878 * want to do a page flip on a single CRTC. That can be tested by 879 * doing a modetest -v first on HDMI1 and then on HDMI0. 880 * 881 * - Since we need the pixelvalve to be disabled and enabled back when 882 * the FIFO is changed, we should keep the FIFO assigned for as long 883 * as the CRTC is enabled, only considering it free again once that 884 * CRTC has been disabled. This can be tested by booting X11 on a 885 * single display, and changing the resolution down and then back up. 886 */ 887 static int vc4_pv_muxing_atomic_check(struct drm_device *dev, 888 struct drm_atomic_state *state) 889 { 890 struct vc4_hvs_state *hvs_new_state; 891 struct drm_crtc_state *old_crtc_state, *new_crtc_state; 892 struct drm_crtc *crtc; 893 unsigned int unassigned_channels; 894 unsigned int i; 895 896 hvs_new_state = vc4_hvs_get_global_state(state); 897 if (!hvs_new_state) 898 return -EINVAL; 899 900 for (i = 0; i < HVS_NUM_CHANNELS; i++) 901 if (!hvs_new_state->fifo_state[i].in_use) > 902 unassigned_channels |= BIT(i); 903 904 for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) { 905 struct vc4_crtc_state *old_vc4_crtc_state = 906 to_vc4_crtc_state(old_crtc_state); 907 struct vc4_crtc_state *new_vc4_crtc_state = 908 to_vc4_crtc_state(new_crtc_state); 909 struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); 910 unsigned int matching_channels; 911 unsigned int channel; 912 913 /* Nothing to do here, let's skip it */ 914 if (old_crtc_state->enable == new_crtc_state->enable) 915 continue; 916 917 /* Muxing will need to be modified, mark it as such */ 918 new_vc4_crtc_state->update_muxing = true; 919 920 /* If we're disabling our CRTC, we put back our channel */ 921 if (!new_crtc_state->enable) { 922 channel = old_vc4_crtc_state->assigned_channel; 923 hvs_new_state->fifo_state[channel].in_use = false; 924 new_vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED; 925 continue; 926 } 927 928 /* 929 * The problem we have to solve here is that we have 930 * up to 7 encoders, connected to up to 6 CRTCs. 931 * 932 * Those CRTCs, depending on the instance, can be 933 * routed to 1, 2 or 3 HVS FIFOs, and we need to set 934 * the change the muxing between FIFOs and outputs in 935 * the HVS accordingly. 936 * 937 * It would be pretty hard to come up with an 938 * algorithm that would generically solve 939 * this. However, the current routing trees we support 940 * allow us to simplify a bit the problem. 941 * 942 * Indeed, with the current supported layouts, if we 943 * try to assign in the ascending crtc index order the 944 * FIFOs, we can't fall into the situation where an 945 * earlier CRTC that had multiple routes is assigned 946 * one that was the only option for a later CRTC. 947 * 948 * If the layout changes and doesn't give us that in 949 * the future, we will need to have something smarter, 950 * but it works so far. 951 */ 952 matching_channels = unassigned_channels & vc4_crtc->data->hvs_available_channels; 953 if (!matching_channels) 954 return -EINVAL; 955 956 channel = ffs(matching_channels) - 1; 957 new_vc4_crtc_state->assigned_channel = channel; 958 unassigned_channels &= ~BIT(channel); 959 hvs_new_state->fifo_state[channel].in_use = true; 960 } 961 962 return 0; 963 } 964 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx
Attachment:
.config.gz
Description: application/gzip
