Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195
On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote: Trim the replies and remove unneeded context. You want to get the attention of other people, not force them to read entire email. >> + mediatek,gce-client-reg:>> +$ref: >> /schemas/types.yaml#/definitions/phandle-array >> +description: The register of display function block to be set by >> gce. >> + There are 4 arguments in this property, gce node, subsys id, >> offset and >> + register size. The subsys id is defined in the gce header of >> each chips >> + include/dt-bindings/gce/-gce.h, mapping to the register >> of display >> + function block. >> +items: >> + items: >> +- description: phandle of GCE >> +- description: GCE subsys id >> +- description: register offset >> +- description: register size >> +minItems: 7 >> +maxItems: 7 >> + > > Hi Rob and krzysztof, > > I got the two messages when running dt_binding_check [1]. This binding > patch was sent previously in [2]. > > If I remove the following items/minItems/maxItems in the mediatek,gce- > client property, the two message disappear. I don't know what's wrong > with the original syntax. Do you have any suggestions for this? > > -items: > - items: > -- description: phandle of GCE > -- description: GCE subsys id > -- description: register offset > -- description: register size > -minItems: 7 > -maxItems: 7 > > > [1]. > Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.examp > le.dtb > /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr > ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: > hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7, 16384, > 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096, > 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7, > 45056, 4096, 4294967295, 7, 49152, 4096] is too long > From schema: This looks like known issue with phandles with variable number of arguments. Either we add it to the exceptions or just define it in reduced way like in other cases - only maxItems: 1 without describing items. Best regards, Krzysztof
[PATCH] fbdev: nvidia: Fix potential divide by zero
variable var->pixclock can be set by user. In case it equals to zero, divide by zero would occur in nvidiafb_set_par. Similar crashes have happened in other fbdev drivers. There is no check and modification on var->pixclock along the call chain to nvidia_check_var and nvidiafb_set_par. We believe it could also be triggered in driver nvidia from user site. Signed-off-by: Wei Chen --- drivers/video/fbdev/nvidia/nvidia.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/nvidia/nvidia.c b/drivers/video/fbdev/nvidia/nvidia.c index e60a276b4855..ea4ba3dfb96b 100644 --- a/drivers/video/fbdev/nvidia/nvidia.c +++ b/drivers/video/fbdev/nvidia/nvidia.c @@ -764,6 +764,8 @@ static int nvidiafb_check_var(struct fb_var_screeninfo *var, int pitch, err = 0; NVTRACE_ENTER(); + if (!var->pixclock) + return -EINVAL; var->transp.offset = 0; var->transp.length = 0; -- 2.25.1
Re: [PATCH 6/7] dt-bindings: display: add rotation property to sitronix,st7789v
On 14/03/2023 12:56, Gerald Loacker wrote: > From: Michael Riesch > > The sitronix-st7789v driver now considers the rotation property. > Add the property to the documentation. > > Signed-off-by: Michael Riesch > Signed-off-by: Gerald Loacker > --- Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 7/7] dt-bindings: display: add panel-timing property to sitronix,st7789v
On 14/03/2023 12:56, Gerald Loacker wrote: > The sitronix-st7789v driver now considers the panel-timing property. > Add the property to the documentation. > > Signed-off-by: Gerald Loacker Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 1/2] dt-bindings: display: seiko,43wvf1g: Add the 'enable-gpios' property
On 14/03/2023 12:17, Fabio Estevam wrote: > Add an optional 'enable-gpios' property that can be used to turn on/off > the display. > > Signed-off-by: Fabio Estevam > --- Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 15/21] dt-bindings: soc: mediatek: add display mutex for MT8365 SoC
Le ven. 10 mars 2023 à 09:39, Krzysztof Kozlowski a écrit : > > On 09/03/2023 15:23, Alexandre Mergnat wrote: > > Add compatible for the MT8365 SoC. > > > > Signed-off-by: Alexandre Mergnat > > --- > > Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git > > a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml > > b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml > > index ca0ca549257d..931d66893dff 100644 > > --- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml > > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml > > @@ -34,6 +34,7 @@ properties: > >- mediatek,mt8186-mdp3-mutex > >- mediatek,mt8192-disp-mutex > >- mediatek,mt8195-disp-mutex > > + - mediatek,mt8365-disp-mutex > > All these look compatible. Either make mt8635 compatible with something > or even rework entire list and make everything compatible. All of them use "mtk_mutec.c" but have specific data. .data = &mt8173_mutex_driver_data}, { .compatible = "mediatek,mt8183-disp-mutex", .data = &mt8183_mutex_driver_data}, { .compatible = "mediatek,mt8186-disp-mutex", .data = &mt8186_mutex_driver_data}, { .compatible = "mediatek,mt8186-mdp3-mutex", .data = &mt8186_mdp_mutex_driver_data}, { .compatible = "mediatek,mt8192-disp-mutex", .data = &mt8192_mutex_driver_data}, { .compatible = "mediatek,mt8195-disp-mutex", .data = &mt8195_mutex_driver_data}, { .compatible = "mediatek,mt8365-disp-mutex", .data = &mt8365_mutex_driver_data}, I don't see any SoC compatible with mt8635, and I have no idea which rework you have in mind. Can you please point me an example ? Thanks Alex
Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
Hi Niklas, On Tue, Mar 14, 2023 at 1:13 PM Niklas Schnelle wrote: > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > not being declared. We thus need to add HAS_IOPORT as dependency for > those drivers using them and guard inline code in headers. > > Co-developed-by: Arnd Bergmann > Signed-off-by: Niklas Schnelle Thanks for your patch! > --- a/drivers/video/fbdev/Kconfig > +++ b/drivers/video/fbdev/Kconfig > @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT > > config FB_ATY > tristate "ATI Mach64 display support" if PCI || ATARI > - depends on FB && !SPARC32 > + depends on FB && HAS_IOPORT && !SPARC32 On Atari, this works without ATARI_ROM_ISA, hence it must not depend on HAS_IOPORT. The only call to inb() is inside a section protected by #ifdef CONFIG_PCI. So: depends on FB && !SPARC32 depends on ATARI || HAS_IOPORT > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH 15/21] dt-bindings: soc: mediatek: add display mutex for MT8365 SoC
Maybe "oneOf" should be added at least ? compatible: oneOf: - enum: Regards, Alex Le mer. 15 mars 2023 à 09:01, Alexandre Mergnat a écrit : > > Le ven. 10 mars 2023 à 09:39, Krzysztof Kozlowski > a écrit : > > > > On 09/03/2023 15:23, Alexandre Mergnat wrote: > > > Add compatible for the MT8365 SoC. > > > > > > Signed-off-by: Alexandre Mergnat > > > --- > > > Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml > > > b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml > > > index ca0ca549257d..931d66893dff 100644 > > > --- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml > > > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml > > > @@ -34,6 +34,7 @@ properties: > > >- mediatek,mt8186-mdp3-mutex > > >- mediatek,mt8192-disp-mutex > > >- mediatek,mt8195-disp-mutex > > > + - mediatek,mt8365-disp-mutex > > > > All these look compatible. Either make mt8635 compatible with something > > or even rework entire list and make everything compatible. > > All of them use "mtk_mutec.c" but have specific data. > > .data = &mt8173_mutex_driver_data}, > { .compatible = "mediatek,mt8183-disp-mutex", > .data = &mt8183_mutex_driver_data}, > { .compatible = "mediatek,mt8186-disp-mutex", > .data = &mt8186_mutex_driver_data}, > { .compatible = "mediatek,mt8186-mdp3-mutex", > .data = &mt8186_mdp_mutex_driver_data}, > { .compatible = "mediatek,mt8192-disp-mutex", > .data = &mt8192_mutex_driver_data}, > { .compatible = "mediatek,mt8195-disp-mutex", > .data = &mt8195_mutex_driver_data}, > { .compatible = "mediatek,mt8365-disp-mutex", > .data = &mt8365_mutex_driver_data}, > > I don't see any SoC compatible with mt8635, and I have no idea which > rework you have in mind. Can you please point me an example ? > > Thanks > Alex
[PATCH] drm/rockchip: vop2: add error check to devm_regmap_init_mmio
--- drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c index 03ca32cd2050..4bde2d354462 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c @@ -2680,6 +2680,8 @@ static int vop2_bind(struct device *dev, struct device *master, void *data) vop2->len = resource_size(res); vop2->map = devm_regmap_init_mmio(dev, vop2->regs, &vop2_regmap_config); + if (IS_ERR(vop2->map)) + return PTR_ERR(vop2->map); ret = vop2_win_init(vop2); if (ret) -- 2.39.2
Re: [PATCH v3 1/6] drm/rockchip: vop2: initialize possible_crtcs properly
Hi Nathan, On 3/14/23 17:08, Nathan Chancellor wrote: > Hi Michael, > > On Tue, Jan 24, 2023 at 06:47:01AM +0100, Michael Riesch wrote: >> The variable possible_crtcs is only initialized for primary and >> overlay planes. Since the VOP2 driver only supports these plane >> types at the moment, the current code is safe. However, in order >> to provide a future-proof solution, fix the initialization of >> the variable. >> >> Reported-by: kernel test robot >> Reported-by: Dan Carpenter >> Signed-off-by: Michael Riesch >> --- >> v3: >> - no changes >> v2: >> - new patch >> >> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 7 --- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >> index 8cecf81a5ae0..374ef821b453 100644 >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c >> @@ -2322,10 +2322,11 @@ static int vop2_create_crtc(struct vop2 *vop2) >> /* change the unused primary window to overlay >> window */ >> win->type = DRM_PLANE_TYPE_OVERLAY; >> } >> -} >> - >> -if (win->type == DRM_PLANE_TYPE_OVERLAY) >> +} else if (win->type == DRM_PLANE_TYPE_OVERLAY) { >> possible_crtcs = (1 << nvps) - 1; >> +} else { >> +possible_crtcs = 0; >> +} >> >> ret = vop2_plane_init(vop2, win, possible_crtcs); >> if (ret) { >> -- >> 2.30.2 >> > > This patch is now in -next as commit 368419a2d429 ("drm/rockchip: vop2: > initialize possible_crtcs properly") and it actually appears to > introduce a path where possible_crtcs could be used uninitialized. > > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2316:8: error: variable > 'possible_crtcs' is used uninitialized whenever 'if' condition is false > [-Werror,-Wsometimes-uninitialized] > if (vp) { > ^~ > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2330:36: note: uninitialized > use occurs here > ret = vop2_plane_init(vop2, win, possible_crtcs); >^~ > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2316:4: note: remove the 'if' > if its condition is always true > if (vp) { > ^~~~ > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2298:21: note: initialize the > variable 'possible_crtcs' to silence this warning > u32 possible_crtcs; > ^ > = 0 > 1 error generated. > > Prior to this change, if that else path was hit, clang recognized based on > the assignment that the next if statement would always be true. Now, if > the else path is taken, the possible_crtcs assignment will be missed. Is > that intentional? As it turns out, the approach in my patch does not cover all paths. I'll submit a follow-up patch that initializes possible_crtcs = 0 and drops the else path. This should solve the issue for good. Regards, Michael
Re: [PATCH 2/2] usb: gadget: functionfs: Add DMABUF import interface
Hi Paul, I love your patch! Perhaps something to improve: [auto build test WARNING on usb/usb-testing] [also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.3-rc2 next-20230315] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing patch link: https://lore.kernel.org/r/20230314105257.17345-3-paul%40crapouillou.net patch subject: [PATCH 2/2] usb: gadget: functionfs: Add DMABUF import interface config: s390-randconfig-r002-20230312 (https://download.01.org/0day-ci/archive/20230315/202303151639.4xupfzjd-...@intel.com/config) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7) 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 s390 cross compiling tool for clang build # apt-get install binutils-s390x-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/4ee364ed5d112c4550344fd037f4e1ef7cc41878 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Paul-Cercueil/usb-gadget-Support-already-mapped-DMA-SGs/20230314-185522 git checkout 4ee364ed5d112c4550344fd037f4e1ef7cc41878 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/media/platform/mediatek/jpeg/ drivers/usb/gadget/function/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot | Link: https://lore.kernel.org/oe-kbuild-all/202303151639.4xupfzjd-...@intel.com/ All warnings (new ones prefixed by >>): In file included from drivers/usb/gadget/function/f_fs.c:18: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __raw_readb(PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x)) ^ include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16' #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x)) ^ In file included from drivers/usb/gadget/function/f_fs.c:18: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) ^ include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32' #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) ^ In file included from drivers/usb/gadget/function/f_fs.c:18: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-poin
Re: [PATCH 1/3] drm/msm/adreno: Add Adreno A690 support
On Tue, 7 Feb 2023 19:40:50 -0800 Bjorn Andersson wrote: > From: Bjorn Andersson > > Introduce support for the Adreno A690, found in Qualcomm SC8280XP. > > Signed-off-by: Bjorn Andersson > Signed-off-by: Bjorn Andersson > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 120 - > drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 34 ++ > drivers/gpu/drm/msm/adreno/adreno_device.c | 14 +++ > drivers/gpu/drm/msm/adreno/adreno_gpu.h| 10 +- > 4 files changed, 173 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index aae60cbd9164..81dfcc5073ad 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -588,6 +588,63 @@ const struct adreno_reglist a660_hwcg[] = { > {}, > }; > > +const struct adreno_reglist a690_hwcg[] = { > + {REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x0222}, > + {REG_A6XX_RBBM_CLOCK_CNTL2_SP0, 0x0220}, > + {REG_A6XX_RBBM_CLOCK_DELAY_SP0, 0x0080}, > + {REG_A6XX_RBBM_CLOCK_HYST_SP0, 0xF3CF}, > + {REG_A6XX_RBBM_CLOCK_CNTL_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_CNTL2_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_CNTL3_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_CNTL4_TP0, 0x0002}, > + {REG_A6XX_RBBM_CLOCK_DELAY_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_DELAY2_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_DELAY3_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_DELAY4_TP0, 0x0001}, > + {REG_A6XX_RBBM_CLOCK_HYST_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_HYST2_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_HYST3_TP0, 0x}, > + {REG_A6XX_RBBM_CLOCK_HYST4_TP0, 0x0007}, > + {REG_A6XX_RBBM_CLOCK_CNTL_RB0, 0x}, > + {REG_A6XX_RBBM_CLOCK_CNTL2_RB0, 0x0100}, > + {REG_A6XX_RBBM_CLOCK_CNTL_CCU0, 0x2220}, > + {REG_A6XX_RBBM_CLOCK_HYST_RB_CCU0, 0x00040F00}, > + {REG_A6XX_RBBM_CLOCK_CNTL_RAC, 0x25222022}, > + {REG_A6XX_RBBM_CLOCK_CNTL2_RAC, 0x}, > + {REG_A6XX_RBBM_CLOCK_DELAY_RAC, 0x0011}, > + {REG_A6XX_RBBM_CLOCK_HYST_RAC, 0x00445044}, > + {REG_A6XX_RBBM_CLOCK_CNTL_TSE_RAS_RBBM, 0x0422}, > + {REG_A6XX_RBBM_CLOCK_MODE_VFD, 0x}, > + {REG_A6XX_RBBM_CLOCK_MODE_GPC, 0x0022}, > + {REG_A6XX_RBBM_CLOCK_DELAY_HLSQ_2, 0x0002}, > + {REG_A6XX_RBBM_CLOCK_MODE_HLSQ, 0x}, > + {REG_A6XX_RBBM_CLOCK_DELAY_TSE_RAS_RBBM, 0x4000}, > + {REG_A6XX_RBBM_CLOCK_DELAY_VFD, 0x}, > + {REG_A6XX_RBBM_CLOCK_DELAY_GPC, 0x0200}, > + {REG_A6XX_RBBM_CLOCK_DELAY_HLSQ, 0x}, > + {REG_A6XX_RBBM_CLOCK_HYST_TSE_RAS_RBBM, 0x}, > + {REG_A6XX_RBBM_CLOCK_HYST_VFD, 0x}, > + {REG_A6XX_RBBM_CLOCK_HYST_GPC, 0x04104004}, > + {REG_A6XX_RBBM_CLOCK_HYST_HLSQ, 0x}, > + {REG_A6XX_RBBM_CLOCK_CNTL_TEX_FCHE, 0x0222}, > + {REG_A6XX_RBBM_CLOCK_DELAY_TEX_FCHE, 0x0111}, > + {REG_A6XX_RBBM_CLOCK_HYST_TEX_FCHE, 0x}, > + {REG_A6XX_RBBM_CLOCK_CNTL_UCHE, 0x}, > + {REG_A6XX_RBBM_CLOCK_HYST_UCHE, 0x0004}, > + {REG_A6XX_RBBM_CLOCK_DELAY_UCHE, 0x0002}, > + {REG_A6XX_RBBM_CLOCK_CNTL, 0x8AA8AA82}, > + {REG_A6XX_RBBM_ISDB_CNT, 0x0182}, > + {REG_A6XX_RBBM_RAC_THRESHOLD_CNT, 0x}, > + {REG_A6XX_RBBM_SP_HYST_CNT, 0x}, > + {REG_A6XX_RBBM_CLOCK_CNTL_GMU_GX, 0x0222}, > + {REG_A6XX_RBBM_CLOCK_DELAY_GMU_GX, 0x0111}, > + {REG_A6XX_RBBM_CLOCK_HYST_GMU_GX, 0x0555}, > + {REG_A6XX_GPU_GMU_AO_GMU_CGC_MODE_CNTL, 0x20200}, > + {REG_A6XX_GPU_GMU_AO_GMU_CGC_DELAY_CNTL, 0x10111}, > + {REG_A6XX_GPU_GMU_AO_GMU_CGC_HYST_CNTL, 0x}, > + {} > +}; > + > static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state) > { > struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > @@ -747,6 +804,45 @@ static const u32 a660_protect[] = { > A6XX_PROTECT_NORDWR(0x1f8c0, 0x), /* note: infinite range */ > }; > > +/* These are for a690 */ > +static const u32 a690_protect[] = { > + A6XX_PROTECT_RDONLY(0x0, 0x004ff), > + A6XX_PROTECT_RDONLY(0x00501, 0x1), > + A6XX_PROTECT_RDONLY(0x0050b, 0x002f4), > + A6XX_PROTECT_NORDWR(0x0050e, 0x0), > + A6XX_PROTECT_NORDWR(0x00510, 0x0), > + A6XX_PROTECT_NORDWR(0x00534, 0x0), > + A6XX_PROTECT_NORDWR(0x00800, 0x00082), > + A6XX_PROTECT_NORDWR(0x008a0, 0x8), > + A6XX_PROTECT_NORDWR(0x008ab, 0x00024), > + A6XX_PROTECT_RDONLY(0x008d0, 0x000bc), > + A6XX_PROTECT_NORDWR(0x00900, 0x0004d), > + A6XX_PROTECT_NORDWR(0x0098d, 0x00272), > + A6XX_PROTECT_NORDWR(0x00e00, 0x1), > + A6XX_PROTECT_NORDWR(0x00e03, 0xc), > + A6XX_PROTECT_NORDWR(0x03c00, 0x000c3), > + A6XX_PROTECT_RDONLY(0x03cc4, 0x01fff), > + A6XX_PROTECT_NORDWR(0x08630, 0x001cf), > + A6XX_PROTECT_NORDWR(0x08e00, 0x0), > + A6XX_PROTECT_NORDWR(0x08e08, 0x00
Re: [PATCH] MAINTAINERS: Add include/drm/drm_bridge.h to DRM DRIVERS FOR BRIDGE CHIPS
Hi, On Mon, 13 Mar 2023 13:59:51 +0800, Liu Ying wrote: > Appropriate maintainers should be suggested for changes to the > include/drm/drm_bridge.h header file, so add the header file to the > 'DRM DRIVERS FOR BRIDGE CHIPS' section. > > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next-fixes) [1/1] MAINTAINERS: Add include/drm/drm_bridge.h to DRM DRIVERS FOR BRIDGE CHIPS https://cgit.freedesktop.org/drm/drm-misc/commit/?id=b28ee4476219b47604b860bd579bd9ce3979a67e -- Neil
[PATCH] drm/rockchip: vop2: fix initialization of possible_crtcs variable
The variable possible_crtcs is not initialized properly since commit 604be85547ce ("drm/rockchip: Add VOP2 driver"). A first attempt to fix this issue has been made in commit 368419a2d429 ("drm/rockchip: vop2: initialize possible_crtcs properly") but turned out to be incomplete. Initialize the variable with zero to cover all possible paths. Fixes: 368419a2d429 ("drm/rockchip: vop2: initialize possible_crtcs properly") Reported-by: kernel test robot Reported-by: Nathan Chancellor Signed-off-by: Michael Riesch --- drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c index 1667e5324b29..0569f1211d9b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c @@ -2301,7 +2301,7 @@ static int vop2_create_crtcs(struct vop2 *vop2) nvp = 0; for (i = 0; i < vop2->registered_num_wins; i++) { struct vop2_win *win = &vop2->win[i]; - u32 possible_crtcs; + u32 possible_crtcs = 0; if (vop2->data->soc_id == 3566) { /* @@ -2329,8 +2329,6 @@ static int vop2_create_crtcs(struct vop2 *vop2) } } else if (win->type == DRM_PLANE_TYPE_OVERLAY) { possible_crtcs = (1 << nvps) - 1; - } else { - possible_crtcs = 0; } ret = vop2_plane_init(vop2, win, possible_crtcs); -- 2.30.2
Re: [PATCH 1/3] drm/msm/adreno: Add Adreno A690 support
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c > > b/drivers/gpu/drm/msm/adreno/adreno_device.c > > index ca38b837dedb..437515e46e5a 100644 > > --- a/drivers/gpu/drm/msm/adreno/adreno_device.c > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c > > @@ -355,6 +355,20 @@ static const struct adreno_info gpulist[] = { > > .init = a6xx_gpu_init, > > .zapfw = "a640_zap.mdt", > > .hwcg = a640_hwcg, > > + }, { > > + .rev = ADRENO_REV(6, 9, 0, ANY_ID), > > + .revn = 690, > > + .name = "A690", > > + .fw = { > > + [ADRENO_FW_SQE] = "a660_sqe.fw", > > + [ADRENO_FW_GMU] = "a690_gmu.bin", > > + }, > > + .gmem = SZ_4M, > > + .inactive_period = DRM_MSM_INACTIVE_PERIOD, > > + .init = a6xx_gpu_init, > > + .zapfw = "a690_zap.mdt", > > + .hwcg = a690_hwcg, > > + .address_space_size = SZ_16G, > > }, > > }; > > This needs > > MODULE_FIRMWARE("qcom/a660_sqe.fw"); > MODULE_FIRMWARE("qcom/a690_gmu.bin"); > MODULE_FIRMWARE("qcom/a690_zap.mbn"); Eek. That should be MODULE_FIRMWARE("qcom/a690_zap.mdt"); > > ...Juerg > > > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > index b4f9b1343d63..da29bd392388 100644 > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h > > @@ -55,7 +55,7 @@ struct adreno_reglist { > > u32 value; > > }; > > > > -extern const struct adreno_reglist a615_hwcg[], a630_hwcg[], a640_hwcg[], > > a650_hwcg[], a660_hwcg[]; > > +extern const struct adreno_reglist a615_hwcg[], a630_hwcg[], a640_hwcg[], > > a650_hwcg[], a660_hwcg[], a690_hwcg[]; > > > > struct adreno_info { > > struct adreno_rev rev; > > @@ -272,6 +272,11 @@ static inline int adreno_is_a660(struct adreno_gpu > > *gpu) > > return gpu->revn == 660; > > } > > > > +static inline int adreno_is_a690(struct adreno_gpu *gpu) > > +{ > > + return gpu->revn == 690; > > +}; > > + > > /* check for a615, a616, a618, a619 or any derivatives */ > > static inline int adreno_is_a615_family(struct adreno_gpu *gpu) > > { > > @@ -286,7 +291,8 @@ static inline int adreno_is_a660_family(struct > > adreno_gpu *gpu) > > /* check for a650, a660, or any derivatives */ > > static inline int adreno_is_a650_family(struct adreno_gpu *gpu) > > { > > - return gpu->revn == 650 || gpu->revn == 620 || > > adreno_is_a660_family(gpu); > > + return gpu->revn == 650 || gpu->revn == 620 || gpu->revn == 690 || > > + adreno_is_a660_family(gpu); > > } > > > > u64 adreno_private_address_space_size(struct msm_gpu *gpu); > pgpsQScJaRoYb.pgp Description: OpenPGP digital signature
[PATCH] fbdev: lxfb: Fix potential divide by zero
var->pixclock can be assigned to zero by user. Without proper check, divide by zero would occur in lx_set_clock. Error out if var->pixclock is zero. Signed-off-by: Wei Chen --- drivers/video/fbdev/geode/lxfb_core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/video/fbdev/geode/lxfb_core.c b/drivers/video/fbdev/geode/lxfb_core.c index 8130e9eee2b4..556d8b1a9e06 100644 --- a/drivers/video/fbdev/geode/lxfb_core.c +++ b/drivers/video/fbdev/geode/lxfb_core.c @@ -235,6 +235,9 @@ static void get_modedb(struct fb_videomode **modedb, unsigned int *size) static int lxfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info) { + if (!var->pixclock) + return -EINVAL; + if (var->xres > 1920 || var->yres > 1440) return -EINVAL; -- 2.25.1
Re: [PATCH] drm/bridge: adv7511: fix race condition bug in adv7511_remove due to unfinished work
Hi, On 08/03/2023 18:34, Zheng Wang wrote: In adv7511_probe, adv7511->hpd_work is bound with adv7511_hpd_work. If we call adv7511_remove with a unfinished work. There may be a race condition where bridge->hpd_mutex was destroyed by drm_bridge_remove and used in adv7511_hpd_work in drm_bridge_hpd_notify. Fix it by canceling the work before cleanup in adv7511_remove. Can you add the relevant Fixes tag ? Thanks, Neil Signed-off-by: Zheng Wang --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index ddceafa7b637..9bf72dd6c1d3 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -1349,6 +1349,7 @@ static void adv7511_remove(struct i2c_client *i2c) { struct adv7511 *adv7511 = i2c_get_clientdata(i2c); + cancel_work_sync(&adv7511->hpd_work); adv7511_uninit_regulators(adv7511); drm_bridge_remove(&adv7511->bridge);
Re: [PATCH 2/2] drm/panel: support for STARRY 2081101QFH032011-53G MIPI-DSI panel
On 14/03/2023 10:05, Ruihai Zhou wrote: The STARRY 2081101QFH032011-53G is a 10.1" WUXGA TFT LCD panel, which fits in nicely with the existing panel-boe-tv101wum-nl6 driver. Hence, we add a new compatible with panel specific config. Signed-off-by: Ruihai Zhou --- .../gpu/drm/panel/panel-boe-tv101wum-nl6.c| 126 ++ 1 file changed, 126 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c index c924f1124ebc..783234ae0f57 100644 --- a/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c +++ b/drivers/gpu/drm/panel/panel-boe-tv101wum-nl6.c @@ -1131,6 +1131,103 @@ static const struct panel_init_cmd auo_b101uan08_3_init_cmd[] = { {}, }; +static const struct panel_init_cmd starry_qfh032011_53g_init_cmd[] = { + _INIT_DCS_CMD(0xB0, 0x01), + _INIT_DCS_CMD(0xC3, 0x4F), + _INIT_DCS_CMD(0xC4, 0x40), + _INIT_DCS_CMD(0xC5, 0x40), + _INIT_DCS_CMD(0xC6, 0x40), + _INIT_DCS_CMD(0xC7, 0x40), + _INIT_DCS_CMD(0xC8, 0x4D), + _INIT_DCS_CMD(0xC9, 0x52), + _INIT_DCS_CMD(0xCA, 0x51), + _INIT_DCS_CMD(0xCD, 0x5D), + _INIT_DCS_CMD(0xCE, 0x5B), + _INIT_DCS_CMD(0xCF, 0x4B), + _INIT_DCS_CMD(0xD0, 0x49), + _INIT_DCS_CMD(0xD1, 0x47), + _INIT_DCS_CMD(0xD2, 0x45), + _INIT_DCS_CMD(0xD3, 0x41), + _INIT_DCS_CMD(0xD7, 0x50), + _INIT_DCS_CMD(0xD8, 0x40), + _INIT_DCS_CMD(0xD9, 0x40), + _INIT_DCS_CMD(0xDA, 0x40), + _INIT_DCS_CMD(0xDB, 0x40), + _INIT_DCS_CMD(0xDC, 0x4E), + _INIT_DCS_CMD(0xDD, 0x52), + _INIT_DCS_CMD(0xDE, 0x51), + _INIT_DCS_CMD(0xE1, 0x5E), + _INIT_DCS_CMD(0xE2, 0x5C), + _INIT_DCS_CMD(0xE3, 0x4C), + _INIT_DCS_CMD(0xE4, 0x4A), + _INIT_DCS_CMD(0xE5, 0x48), + _INIT_DCS_CMD(0xE6, 0x46), + _INIT_DCS_CMD(0xE7, 0x42), + _INIT_DCS_CMD(0xB0, 0x03), + _INIT_DCS_CMD(0xBE, 0x03), + _INIT_DCS_CMD(0xCC, 0x44), + _INIT_DCS_CMD(0xC8, 0x07), + _INIT_DCS_CMD(0xC9, 0x05), + _INIT_DCS_CMD(0xCA, 0x42), + _INIT_DCS_CMD(0xCD, 0x3E), + _INIT_DCS_CMD(0xCF, 0x60), + _INIT_DCS_CMD(0xD2, 0x04), + _INIT_DCS_CMD(0xD3, 0x04), + _INIT_DCS_CMD(0xD4, 0x01), + _INIT_DCS_CMD(0xD5, 0x00), + _INIT_DCS_CMD(0xD6, 0x03), + _INIT_DCS_CMD(0xD7, 0x04), + _INIT_DCS_CMD(0xD9, 0x01), + _INIT_DCS_CMD(0xDB, 0x01), + _INIT_DCS_CMD(0xE4, 0xF0), + _INIT_DCS_CMD(0xE5, 0x0A), + _INIT_DCS_CMD(0xB0, 0x00), + _INIT_DCS_CMD(0xCC, 0x08), + _INIT_DCS_CMD(0xC2, 0x08), + _INIT_DCS_CMD(0xC4, 0x10), + _INIT_DCS_CMD(0xB0, 0x02), + _INIT_DCS_CMD(0xC0, 0x00), + _INIT_DCS_CMD(0xC1, 0x0A), + _INIT_DCS_CMD(0xC2, 0x20), + _INIT_DCS_CMD(0xC3, 0x24), + _INIT_DCS_CMD(0xC4, 0x23), + _INIT_DCS_CMD(0xC5, 0x29), + _INIT_DCS_CMD(0xC6, 0x23), + _INIT_DCS_CMD(0xC7, 0x1C), + _INIT_DCS_CMD(0xC8, 0x19), + _INIT_DCS_CMD(0xC9, 0x17), + _INIT_DCS_CMD(0xCA, 0x17), + _INIT_DCS_CMD(0xCB, 0x18), + _INIT_DCS_CMD(0xCC, 0x1A), + _INIT_DCS_CMD(0xCD, 0x1E), + _INIT_DCS_CMD(0xCE, 0x20), + _INIT_DCS_CMD(0xCF, 0x23), + _INIT_DCS_CMD(0xD0, 0x07), + _INIT_DCS_CMD(0xD1, 0x00), + _INIT_DCS_CMD(0xD2, 0x00), + _INIT_DCS_CMD(0xD3, 0x0A), + _INIT_DCS_CMD(0xD4, 0x13), + _INIT_DCS_CMD(0xD5, 0x1C), + _INIT_DCS_CMD(0xD6, 0x1A), + _INIT_DCS_CMD(0xD7, 0x13), + _INIT_DCS_CMD(0xD8, 0x17), + _INIT_DCS_CMD(0xD9, 0x1C), + _INIT_DCS_CMD(0xDA, 0x19), + _INIT_DCS_CMD(0xDB, 0x17), + _INIT_DCS_CMD(0xDC, 0x17), + _INIT_DCS_CMD(0xDD, 0x18), + _INIT_DCS_CMD(0xDE, 0x1A), + _INIT_DCS_CMD(0xDF, 0x1E), + _INIT_DCS_CMD(0xE0, 0x20), + _INIT_DCS_CMD(0xE1, 0x23), + _INIT_DCS_CMD(0xE2, 0x07), + _INIT_DCS_CMD(0X11), + _INIT_DELAY_CMD(120), + _INIT_DCS_CMD(0X29), + _INIT_DELAY_CMD(80), + {}, +}; + static inline struct boe_panel *to_boe_panel(struct drm_panel *panel) { return container_of(panel, struct boe_panel, base); @@ -1497,6 +1594,32 @@ static const struct panel_desc boe_tv105wum_nw0_desc = { .init_cmds = boe_init_cmd, }; +static const struct drm_display_mode starry_qfh032011_53g_default_mode = { + .clock = 165731, + .hdisplay = 1200, + .hsync_start = 1200 + 100, + .hsync_end = 1200 + 100 + 10, + .htotal = 1200 + 100 + 10 + 100, + .vdisplay = 1920, + .vsync_start = 1920 + 14, + .vsync_end = 1920 + 14 + 10, + .vtotal = 1920 + 14 + 10 + 15, +}; + +static const struct panel_desc starry_qfh032011_53g_desc = { + .modes = &starry_qfh032011_53g_default_mode, + .bpc = 8, + .size = { + .width_mm = 135, + .height_mm = 216, + }, + .lanes = 4, + .format = MIPI_DSI_FMT_RGB888,
Re: [PATCH 2/2] drm/panel: seiko-43wvf1g: Add the 'enable-gpios' property
On 14/03/2023 12:17, Fabio Estevam wrote: Sometimes a GPIO is needed to turn on/off the display. Add support for this usecase by introducing the optional 'enable-gpios' property. Tested on a imx53qsb board. Signed-off-by: Fabio Estevam --- drivers/gpu/drm/panel/panel-seiko-43wvf1g.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c index 76160e5d43bd..c250ca36a5b3 100644 --- a/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c +++ b/drivers/gpu/drm/panel/panel-seiko-43wvf1g.c @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -48,6 +49,7 @@ struct seiko_panel { const struct seiko_panel_desc *desc; struct regulator *dvdd; struct regulator *avdd; + struct gpio_desc *enable_gpio; }; static inline struct seiko_panel *to_seiko_panel(struct drm_panel *panel) @@ -139,6 +141,8 @@ static int seiko_panel_unprepare(struct drm_panel *panel) if (!p->prepared) return 0; + gpiod_set_value_cansleep(p->enable_gpio, 0); + regulator_disable(p->avdd); /* Add a 100ms delay as per the panel datasheet */ @@ -174,6 +178,8 @@ static int seiko_panel_prepare(struct drm_panel *panel) goto disable_dvdd; } + gpiod_set_value_cansleep(p->enable_gpio, 1); + p->prepared = true; return 0; @@ -252,6 +258,12 @@ static int seiko_panel_probe(struct device *dev, if (IS_ERR(panel->avdd)) return PTR_ERR(panel->avdd); + panel->enable_gpio = devm_gpiod_get_optional(dev, "enable", +GPIOD_OUT_LOW); + if (IS_ERR(panel->enable_gpio)) + return dev_err_probe(dev, PTR_ERR(panel->enable_gpio), +"failed to request GPIO\n"); + drm_panel_init(&panel->base, dev, &seiko_panel_funcs, DRM_MODE_CONNECTOR_DPI); Reviewed-by: Neil Armstrong
Re: [PATCH v2] drm/bridge: Fix returned array size name for atomic_get_input_bus_fmts kdoc
Hi, On Tue, 14 Mar 2023 13:50:35 +0800, Liu Ying wrote: > The returned array size for input formats is set through > atomic_get_input_bus_fmts()'s 'num_input_fmts' argument, so use > 'num_input_fmts' to represent the array size in the function's kdoc, > not 'num_output_fmts'. > > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes) [1/1] drm/bridge: Fix returned array size name for atomic_get_input_bus_fmts kdoc https://cgit.freedesktop.org/drm/drm-misc/commit/?id=0d3c9333d976af41d7dbc6bf4d9d2e95fbdf9c89 -- Neil
Re: [PATCH] drm/meson: dw-hdmi: Fix devm_regulator_*get_enable*() conversion again
Hi, On Thu, 09 Mar 2023 16:24:46 +0100, Marek Szyprowski wrote: > devm_regulator_get_enable_optional() returns -ENODEV if requested > optional regulator is not present. Adjust code for that, because in the > 67d0a30128c9 I've incorrectly assumed that it also returns 0 when > regulator is not present. > > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-fixes) [1/1] drm/meson: dw-hdmi: Fix devm_regulator_*get_enable*() conversion again https://cgit.freedesktop.org/drm/drm-misc/commit/?id=4028cbf867f70a3c599c9b0c9509334c56ed97d7 -- Neil
Re: [PATCH 1/2] dt-bindings: display: panel: Add compatible for Starry 2081101QFH032011-53G
Hi, On Tue, 14 Mar 2023 16:50:34 +0800, Ruihai Zhou wrote: > The STARRY 2081101QFH032011-53G is a 10.1" WUXGA TFT LCD panel, > which fits in nicely with the existing panel-boe-tv101wum-nl6 > driver. Hence, we add a new compatible with panel specific config. > > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next-fixes) [1/2] dt-bindings: display: panel: Add compatible for Starry 2081101QFH032011-53G https://cgit.freedesktop.org/drm/drm-misc/commit/?id=c4969c95266c672b5dc0c6a1ac070b6d647fea22 -- Neil
Re: [PATCH 1/2] dt-bindings: display: seiko,43wvf1g: Add the 'enable-gpios' property
Hi, On Tue, 14 Mar 2023 08:17:23 -0300, Fabio Estevam wrote: > Add an optional 'enable-gpios' property that can be used to turn on/off > the display. > > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next-fixes) [1/2] dt-bindings: display: seiko,43wvf1g: Add the 'enable-gpios' property https://cgit.freedesktop.org/drm/drm-misc/commit/?id=1afdbd475adc609d829e560389d33eb90501660f [2/2] drm/panel: seiko-43wvf1g: Add the 'enable-gpios' property https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e2945e6c5111726536c6046eaa1b840636e066a8 -- Neil
Re: [PATCH 2/2] drm/panel: support for STARRY 2081101QFH032011-53G MIPI-DSI panel
Hi, On Tue, 14 Mar 2023 17:05:49 +0800, Ruihai Zhou wrote: > The STARRY 2081101QFH032011-53G is a 10.1" WUXGA TFT LCD panel, > which fits in nicely with the existing panel-boe-tv101wum-nl6 > driver. Hence, we add a new compatible with panel specific config. > > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next-fixes) [2/2] drm/panel: support for STARRY 2081101QFH032011-53G MIPI-DSI panel https://cgit.freedesktop.org/drm/drm-misc/commit/?id=6069b66cd9622c4b29817d4e19737e6f023b909a -- Neil
Re: [PATCH v3] drm/i915/pxp: limit drm-errors or warning on firmware API failures
Hi, Tested the patch with Ubuntu 22.04 desktop + Linux 6.2-rc3 (drm-tip) kernel, on TGL-H HW. With it, this log spam has disappeared: [ 8691.608933] i915 :00:02.0: [drm] PXP firmware failed arb session init request ret=[0x101f] [ 8691.709442] i915 :00:02.0: [drm] *ERROR* arb session failed to go in play There's now just a single message on boot: [ 11.674177] [ cut here ] [ 11.674183] i915 :00:02.0: PXP init-arb-session-15 failed due to BIOS/SOC:0x101a:ERR_PLATFORM_CONFIG [ 11.674222] WARNING: CPU: 12 PID: 123 at drivers/gpu/drm/i915/pxp/intel_pxp_tee.c:345 intel_pxp_tee_cmd_create_arb_session+0x19a/0x1e0 [i915] ... [ 11.674961] Call Trace: [ 11.674966] [ 11.674970] ? pxp_session_work+0x123/0x1d0 [i915] [ 11.675130] pxp_session_work+0x123/0x1d0 [i915] [ 11.675286] process_one_work+0x1d9/0x3e0 [ 11.675295] worker_thread+0x21b/0x3d0 [ 11.675301] ? __pfx_worker_thread+0x10/0x10 [ 11.675307] kthread+0xe9/0x110 [ 11.675311] ? __pfx_kthread+0x10/0x10 [ 11.675315] ret_from_fork+0x29/0x50 [ 11.675323] [ 11.675325] ---[ end trace ]--- - Eero On 14.3.2023 19.58, Alan Previn wrote: MESA driver is creating protected context on every driver handle creation to query caps bits for app. So when running CI tests, they are observing hundreds of drm_errors when enabling PXP in .config but using SOC fusing or BIOS configuration that cannot support PXP sessions. The fixes tag referenced below was to resolve a related issue where we wanted to silence error messages, but that case was due to outdated IFWI (firmware) that definitely needed an upgrade and was, at that point, considered a one-off case as opposed to today's realization that default CI was enabling PXP in kernel config for all testing. So with this patch, let's strike a balance between issues that is critical but are root-caused from HW/platform gaps (louder drm-warn but just ONCE) vs other cases where it could also come from session state machine (which cannot be a WARN_ONCE since it can be triggered due to runtime operation events). Let's use helpers for these so as more functions are added in future features / HW (or as FW designers continue to bless upstreaming of the error codes and meanings), we only need to update the helpers. NOTE: Don't completely remove FW errors (via drm_debug) or else cusomer apps that really needs to know that content protection failed won't be aware of it. v2: - Add fixes tag (Trvtko) v3: - Break multi-line drm_dbg strings into separate drm_dbg (Daniele) - Fix couple of typecasting nits (Daniele) Fixes: b762787bf767 ("drm/i915/pxp: Use drm_dbg if arb session failed due to fw version") Signed-off-by: Alan Previn --- .../i915/pxp/intel_pxp_cmd_interface_cmn.h| 3 + drivers/gpu/drm/i915/pxp/intel_pxp_session.c | 2 +- drivers/gpu/drm/i915/pxp/intel_pxp_tee.c | 77 +++ 3 files changed, 67 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h index ae9b151b7cb7..6f6541d5e49a 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_cmn.h @@ -18,6 +18,9 @@ enum pxp_status { PXP_STATUS_SUCCESS = 0x0, PXP_STATUS_ERROR_API_VERSION = 0x1002, + PXP_STATUS_NOT_READY = 0x100e, + PXP_STATUS_PLATFCONFIG_KF1_NOVERIF = 0x101a, + PXP_STATUS_PLATFCONFIG_KF1_BAD = 0x101f, PXP_STATUS_OP_NOT_PERMITTED = 0x4013 }; diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c index 448cacb0465d..7de849cb6c47 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c @@ -74,7 +74,7 @@ static int pxp_create_arb_session(struct intel_pxp *pxp) ret = pxp_wait_for_session_state(pxp, ARB_SESSION, true); if (ret) { - drm_err(>->i915->drm, "arb session failed to go in play\n"); + drm_dbg(>->i915->drm, "arb session failed to go in play\n"); return ret; } drm_dbg(>->i915->drm, "PXP ARB session is alive\n"); diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c index d9d248b48093..6f89cd850251 100644 --- a/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c +++ b/drivers/gpu/drm/i915/pxp/intel_pxp_tee.c @@ -19,6 +19,37 @@ #include "intel_pxp_tee.h" #include "intel_pxp_types.h" +static bool +is_fw_err_platform_config(u32 type) +{ + switch (type) { + case PXP_STATUS_ERROR_API_VERSION: + case PXP_STATUS_PLATFCONFIG_KF1_NOVERIF: + case PXP_STATUS_PLATFCON
Adreno devfreq lockdep splat with 6.3-rc2
Hi Rob, Since 6.3-rc2 (or possibly -rc1), I'm now seeing the below devfreq-related lockdep splat. I noticed that you posted a fix for something similar here: https://lore.kernel.org/r/20230312204150.1353517-9-robdcl...@gmail.com but that particular patch makes no difference. >From skimming the calltraces below and qos/devfreq related changes in 6.3-rc1 it seems like this could be related to: fadcc3ab1302 ("drm/msm/gpu: Bypass PM QoS constraint for idle clamp") Johan [ 35.389822] == [ 35.389824] WARNING: possible circular locking dependency detected [ 35.389826] 6.3.0-rc2 #208 Not tainted [ 35.389828] -- [ 35.389829] ring0/348 is trying to acquire lock: [ 35.389830] 43424cfa2078 (&devfreq->lock){+.+.}-{3:3}, at: qos_min_notifier_call+0x28/0x88 [ 35.389845] but task is already holding lock: [ 35.389846] 4342432b78e8 (&(c->notifiers)->rwsem){}-{3:3}, at: blocking_notifier_call_chain+0x34/0xa0 [ 35.389855] which lock already depends on the new lock. [ 35.389856] the existing dependency chain (in reverse order) is: [ 35.389857] -> #4 (&(c->notifiers)->rwsem){}-{3:3}: [ 35.389861]lock_acquire+0x68/0x84 [ 35.389865]down_write+0x58/0xfc [ 35.389869]blocking_notifier_chain_register+0x30/0x8c [ 35.389872]freq_qos_add_notifier+0x68/0x7c [ 35.389876]dev_pm_qos_add_notifier+0xe8/0x114 [ 35.389881]devfreq_add_device.part.0+0x360/0x5a4 [ 35.389884]devm_devfreq_add_device+0x74/0xe0 [ 35.389886]msm_devfreq_init+0xa0/0x154 [msm] [ 35.389915]msm_gpu_init+0x320/0x5b0 [msm] [ 35.389933]adreno_gpu_init+0x164/0x2d8 [msm] [ 35.389951]a6xx_gpu_init+0x270/0x608 [msm] [ 35.389968]adreno_bind+0x184/0x284 [msm] [ 35.389983]component_bind_all+0x124/0x288 [ 35.389989]msm_drm_bind+0x1d8/0x6a8 [msm] [ 35.390004]try_to_bring_up_aggregate_device+0x1ec/0x2f4 [ 35.390007]__component_add+0xa8/0x194 [ 35.390010]component_add+0x14/0x20 [ 35.390012]dp_display_probe+0x2b4/0x474 [msm] [ 35.390029]platform_probe+0x68/0xd8 [ 35.390031]really_probe+0x184/0x3c8 [ 35.390034]__driver_probe_device+0x7c/0x188 [ 35.390036]driver_probe_device+0x3c/0x110 [ 35.390039]__device_attach_driver+0xbc/0x158 [ 35.390041]bus_for_each_drv+0x84/0xe0 [ 35.390044]__device_attach+0xa8/0x1d4 [ 35.390046]device_initial_probe+0x14/0x20 [ 35.390049]bus_probe_device+0xac/0xb0 [ 35.390051]deferred_probe_work_func+0xa0/0xf4 [ 35.390053]process_one_work+0x288/0x6c4 [ 35.390056]worker_thread+0x74/0x450 [ 35.390058]kthread+0x118/0x11c [ 35.390060]ret_from_fork+0x10/0x20 [ 35.390063] -> #3 (dev_pm_qos_mtx){+.+.}-{3:3}: [ 35.390066]lock_acquire+0x68/0x84 [ 35.390068]__mutex_lock+0x98/0x428 [ 35.390072]mutex_lock_nested+0x2c/0x38 [ 35.390074]dev_pm_qos_remove_notifier+0x34/0x140 [ 35.390077]genpd_remove_device+0x3c/0x174 [ 35.390081]genpd_dev_pm_detach+0x78/0x1b4 [ 35.390083]dev_pm_domain_detach+0x24/0x34 [ 35.390085]a6xx_gmu_remove+0x64/0xd0 [msm] [ 35.390101]a6xx_destroy+0xa8/0x138 [msm] [ 35.390116]adreno_unbind+0x40/0x64 [msm] [ 35.390131]component_unbind+0x38/0x6c [ 35.390134]component_unbind_all+0xc8/0xd4 [ 35.390136]msm_drm_uninit.isra.0+0x168/0x1dc [msm] [ 35.390152]msm_drm_bind+0x2f4/0x6a8 [msm] [ 35.390167]try_to_bring_up_aggregate_device+0x1ec/0x2f4 [ 35.390170]__component_add+0xa8/0x194 [ 35.390172]component_add+0x14/0x20 [ 35.390175]dp_display_probe+0x2b4/0x474 [msm] [ 35.390190]platform_probe+0x68/0xd8 [ 35.390192]really_probe+0x184/0x3c8 [ 35.390194]__driver_probe_device+0x7c/0x188 [ 35.390197]driver_probe_device+0x3c/0x110 [ 35.390199]__device_attach_driver+0xbc/0x158 [ 35.390201]bus_for_each_drv+0x84/0xe0 [ 35.390203]__device_attach+0xa8/0x1d4 [ 35.390206]device_initial_probe+0x14/0x20 [ 35.390208]bus_probe_device+0xac/0xb0 [ 35.390210]deferred_probe_work_func+0xa0/0xf4 [ 35.390212]process_one_work+0x288/0x6c4 [ 35.390214]worker_thread+0x74/0x450 [ 35.390216]kthread+0x118/0x11c [ 35.390217]ret_from_fork+0x10/0x20 [ 35.390219] -> #2 (&gmu->lock){+.+.}-{3:3}: [ 35.390222]lock_acquire+0x68/0x84 [ 35.390224]__mutex_lock+0x98/0x428 [ 35.390226]mutex_lock_nested+0x2c/0x38 [ 35.390229]a6xx_gpu_set_freq+0x30/0x5c [msm] [ 35.390245]msm_d
Re: [PATCH] drm/bridge: adv7511: fix race condition bug in adv7511_remove due to unfinished work
于2023年3月15日周三 17:08写道: > > Hi, > > On 08/03/2023 18:34, Zheng Wang wrote: > > In adv7511_probe, adv7511->hpd_work is bound with adv7511_hpd_work. > > If we call adv7511_remove with a unfinished work. There may be a > > race condition where bridge->hpd_mutex was destroyed by > > drm_bridge_remove and used in adv7511_hpd_work in drm_bridge_hpd_notify. > > > > Fix it by canceling the work before cleanup in adv7511_remove. > > > > Can you add the relevant Fixes tag ? > Hi Neil, Thanks for your reply and kind reminder. Sorry for my mistake. I'll append more messages in the next version of patch. Best regards, Zheng > Thanks, > Neil > > > Signed-off-by: Zheng Wang > > --- > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > index ddceafa7b637..9bf72dd6c1d3 100644 > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > @@ -1349,6 +1349,7 @@ static void adv7511_remove(struct i2c_client *i2c) > > { > > struct adv7511 *adv7511 = i2c_get_clientdata(i2c); > > > > + cancel_work_sync(&adv7511->hpd_work); > > adv7511_uninit_regulators(adv7511); > > > > drm_bridge_remove(&adv7511->bridge); >
[PATCH] fbdev: au1200fb: Fix potential divide by zero
var->pixclock can be assigned to zero by user. Without proper check, divide by zero would occur when invoking macro PICOS2KHZ in au1200fb_fb_check_var. Error out if var->pixclock is zero. Signed-off-by: Wei Chen --- drivers/video/fbdev/au1200fb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c index 81c315454428..b6b22fa4a8a0 100644 --- a/drivers/video/fbdev/au1200fb.c +++ b/drivers/video/fbdev/au1200fb.c @@ -1040,6 +1040,9 @@ static int au1200fb_fb_check_var(struct fb_var_screeninfo *var, u32 pixclock; int screen_size, plane; + if (!var->pixclock) + return -EINVAL; + plane = fbdev->plane; /* Make sure that the mode respect all LCD controller and -- 2.25.1
Re: [PATCH 07/21] dt-bindings: display: mediatek: dpi: add binding for MT8365
Hi Chun-Kuang, Le lun. 13 mars 2023 à 16:17, Chun-Kuang Hu a écrit : > > Hi, Alexandre: > > Alexandre Mergnat 於 2023年3月9日 週四 下午10:23寫道: > > > > From: Fabien Parent > > > > DPI for MT8365 is compatible with MT8192 but requires an additional > > clock. Modify the documentation to requires this clock only on MT8365 SoCs. > > If MT8365 DPI has additional clock, why it is compatible with MT8192 DPI? > I think some part of MT8165 DPI works under the speed control by the > DPI clock and this is different with MT8192 DPI, how could these two > are compatible? AFAII, The mtk_dpi driver manage the 4th clock as optional dpi->dpi_clk = devm_clk_get_optional(dev, "dpi"); And all configuration variables for mt8192 are the same for mt8365. These configuration clock variables (like cal_factor) aren't correlated with the 4th clock. The clock number doesn't impact the configuration variable because the 4th clock is simply retrieved from DTS, saved in the driver structure and prepare/unprepare/enable/disable, like other clocks. Regards, Alex
Re: [PATCH] drm/komeda: Take over EFI framebuffer properly
On 13/03/2023 10:22, patrik.bergl...@arm.com wrote: > From: Patrik Berglund > > The Arm Morello board EDK2 port already provides an EFI GOP display for > Ceti/Cetus (Komeda) with more boards incoming. > However, once the Komeda driver probes and takes over the hardware, > it should take over the logical framebuffer as well, otherwise, > the now-defunct GOP device hangs around and virtual console output > inevitably disappears into the wrong place most of the time. > > We'll do this right before doing the SRST because that is the point > when the GOP will stop working. > The GOP might also fail because the encoder driver do things but this > is better than nothing. > > Signed-off-by: Patrik Berglund +CC the maintainers. Looks right to me, hdlcd has something very similar. Reviewed-by: Steven Price > --- > drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 12 > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 6 ++ > drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 1 + > 3 files changed, 19 insertions(+) > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c > b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c > index 6c56f5662bc7..72035af9bc5f 100644 > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c > @@ -8,6 +8,7 @@ > #include > #include > #include "d71_dev.h" > +#include "komeda_kms.h" > #include "malidp_io.h" > > static u64 get_lpu_event(struct d71_pipeline *d71_pipeline) > @@ -310,6 +311,17 @@ static int d71_reset(struct d71_dev *d71) > u32 __iomem *gcu = d71->gcu_addr; > int ret; > > + /* > + * If we are already running, the most likely reason is that the EFI > left > + * us running (GOP), so make sure to take over from simple framebuffer > + * drivers. > + */ > + if (malidp_read32(gcu, BLK_STATUS) & GCU_STATUS_ACTIVE) { > + ret = komeda_kms_remove_framebuffers(); > + if (ret) > + return ret; > + } > + > malidp_write32(gcu, BLK_CONTROL, GCU_CONTROL_SRST); > > ret = dp_wait_cond(!(malidp_read32(gcu, BLK_CONTROL) & > GCU_CONTROL_SRST), > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > index 62dc64550793..12af409aeabb 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > @@ -7,6 +7,7 @@ > #include > #include > > +#include > #include > #include > #include > @@ -349,3 +350,8 @@ void komeda_kms_detach(struct komeda_kms_dev *kms) > komeda_kms_cleanup_private_objs(kms); > drm->dev_private = NULL; > } > + > +int komeda_kms_remove_framebuffers(void) > +{ > + return drm_aperture_remove_framebuffers(false, &komeda_kms_driver); > +} > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h > index 3a872c292091..1a43707ed68f 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h > @@ -187,5 +187,6 @@ void komeda_crtc_flush_and_wait_for_flip_done(struct > komeda_crtc *kcrtc, > > struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev); > void komeda_kms_detach(struct komeda_kms_dev *kms); > +int komeda_kms_remove_framebuffers(void); > > #endif /*_KOMEDA_KMS_H_*/
Re: [PATCH 19/21] drm/mediatek: dpi: add support for dpi clock
Le lun. 13 mars 2023 à 16:21, Chun-Kuang Hu a écrit : > > > > > + dpi->dpi_clk = devm_clk_get_optional(dev, "dpi"); > > For MT8365, DPI clock is not optional, so make sure that MT8365 DPI > should have this clock. This should be check and notified at build time thanks to the device-tree binding. Do you prefer this?: #IF MT8365_SOC dpi->dpi_clk = devm_clk_get(dev, "dpi"); Regards, Alex
Re: [PATCH 1/2] drm/i915/pmu: Use functions common with sysfs to read actual freq
On 10/03/2023 00:59, Ashutosh Dixit wrote: Expose intel_rps_read_actual_frequency_fw to read the actual freq without taking forcewake for use by PMU. The code is refactored to use a common set of functions across sysfs and PMU. Using common functions with sysfs in PMU solves the issues of missing support for MTL and missing support for older generations (prior to Gen6). It also future proofs the PMU where sometimes code has been updated for sysfs and PMU has been missed. v2: Remove runtime_pm_if_in_use from read_actual_frequency_fw (Tvrtko) Fixes: 22009b6dad66 ("drm/i915/mtl: Modify CAGF functions for MTL") Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8280 Signed-off-by: Ashutosh Dixit --- drivers/gpu/drm/i915/gt/intel_rps.c | 34 - drivers/gpu/drm/i915/gt/intel_rps.h | 2 +- drivers/gpu/drm/i915/i915_pmu.c | 10 - 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c index 4d0dc9de23f9..9d9ac35691fc 100644 --- a/drivers/gpu/drm/i915/gt/intel_rps.c +++ b/drivers/gpu/drm/i915/gt/intel_rps.c @@ -2046,16 +2046,6 @@ void intel_rps_sanitize(struct intel_rps *rps) rps_disable_interrupts(rps); } -u32 intel_rps_read_rpstat_fw(struct intel_rps *rps) -{ - struct drm_i915_private *i915 = rps_to_i915(rps); - i915_reg_t rpstat; - - rpstat = (GRAPHICS_VER(i915) >= 12) ? GEN12_RPSTAT1 : GEN6_RPSTAT1; - - return intel_uncore_read_fw(rps_to_gt(rps)->uncore, rpstat); -} - u32 intel_rps_read_rpstat(struct intel_rps *rps) { struct drm_i915_private *i915 = rps_to_i915(rps); @@ -2089,10 +2079,11 @@ u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat) return cagf; } -static u32 read_cagf(struct intel_rps *rps) +static u32 __read_cagf(struct intel_rps *rps, bool take_fw) { struct drm_i915_private *i915 = rps_to_i915(rps); struct intel_uncore *uncore = rps_to_uncore(rps); + i915_reg_t r = INVALID_MMIO_REG; u32 freq; /* @@ -2100,22 +2091,30 @@ static u32 read_cagf(struct intel_rps *rps) * registers will return 0 freq when GT is in RC6 */ if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) { - freq = intel_uncore_read(uncore, MTL_MIRROR_TARGET_WP1); + r = MTL_MIRROR_TARGET_WP1; } else if (GRAPHICS_VER(i915) >= 12) { - freq = intel_uncore_read(uncore, GEN12_RPSTAT1); + r = GEN12_RPSTAT1; } else if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) { vlv_punit_get(i915); freq = vlv_punit_read(i915, PUNIT_REG_GPU_FREQ_STS); vlv_punit_put(i915); + goto exit; Alternatively you could avoid the goto by making the read below conditional on r being set. One more conditional though for avoiding gotos.. up to you. } else if (GRAPHICS_VER(i915) >= 6) { - freq = intel_uncore_read(uncore, GEN6_RPSTAT1); + r = GEN6_RPSTAT1; } else { - freq = intel_uncore_read(uncore, MEMSTAT_ILK); + r = MEMSTAT_ILK; } + freq = take_fw ? intel_uncore_read(uncore, r) : intel_uncore_read_fw(uncore, r); +exit: return intel_rps_get_cagf(rps, freq); } +static u32 read_cagf(struct intel_rps *rps) +{ + return __read_cagf(rps, true); +} There is only one caller so up to you if you think a helper is needed or not. + u32 intel_rps_read_actual_frequency(struct intel_rps *rps) { struct intel_runtime_pm *rpm = rps_to_uncore(rps)->rpm; @@ -2128,6 +2127,11 @@ u32 intel_rps_read_actual_frequency(struct intel_rps *rps) return freq; } +u32 intel_rps_read_actual_frequency_fw(struct intel_rps *rps) +{ + return intel_gpu_freq(rps, __read_cagf(rps, false)); +} + u32 intel_rps_read_punit_req(struct intel_rps *rps) { struct intel_uncore *uncore = rps_to_uncore(rps); diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h index c622962c6bef..2d5b3ef58606 100644 --- a/drivers/gpu/drm/i915/gt/intel_rps.h +++ b/drivers/gpu/drm/i915/gt/intel_rps.h @@ -39,6 +39,7 @@ int intel_gpu_freq(struct intel_rps *rps, int val); int intel_freq_opcode(struct intel_rps *rps, int val); u32 intel_rps_get_cagf(struct intel_rps *rps, u32 rpstat1); u32 intel_rps_read_actual_frequency(struct intel_rps *rps); +u32 intel_rps_read_actual_frequency_fw(struct intel_rps *rps); u32 intel_rps_get_requested_frequency(struct intel_rps *rps); u32 intel_rps_get_min_frequency(struct intel_rps *rps); u32 intel_rps_get_min_raw_freq(struct intel_rps *rps); @@ -52,7 +53,6 @@ u32 intel_rps_get_rpn_frequency(struct intel_rps *rps); u32 intel_rps_read_punit_req(struct intel_rps *rps); u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps); u32 intel_rps_read_rpstat(struct intel_rps *rps); -u32 intel_rps_read_
Re: [PATCH 2/2] drm/i915/pmu: Remove fallback to requested freq for SLPC
On 10/03/2023 00:59, Ashutosh Dixit wrote: The fallback to requested freq does not work for SLPC because SLPC does not use 'struct intel_rps'. Also for SLPC requested freq can only be obtained from a hw register after acquiring forcewake which we don't want to do for PMU. Therefore remove fallback to requested freq for SLPC. The actual freq will be 0 when gt is in RC6 which is correct. Also this is rare since PMU freq sampling happens only when gt is unparked. Signed-off-by: Ashutosh Dixit --- drivers/gpu/drm/i915/i915_pmu.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 7ece883a7d95..f697fabed64a 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -393,7 +393,14 @@ frequency_sample(struct intel_gt *gt, unsigned int period_ns) * frequency. Fortunately, the read should rarely fail! */ val = intel_rps_read_actual_frequency_fw(rps); - if (!val) + + /* +* SLPC does not use 'struct intel_rps'. Also for SLPC +* requested freq can only be obtained after acquiring +* forcewake and reading a hw register. For SLPC just +* let val be 0 +*/ + if (!val && !intel_uc_uses_guc_slpc(>->uc)) val = intel_gpu_freq(rps, rps->cur_freq); I really dislike sprinkling of "uses slpc" since I think the thing hasn't really been integrated nicely. Case in point is probably the flow duality in intel_rps_boost. Data structures as well, even though some fields and concepts are shared. For instance why we can't have the notion of software tracked cur_freq in rps, and/or have it zero if with SLPC we can't have it otherwise? I will abstain, sorry. Regards, Tvrtko add_sample_mult(&pmu->sample[__I915_SAMPLE_FREQ_ACT],
Re: [RFC 01/10] drm: Track clients by tgid and not tid
Hi, On 14/03/2023 15:33, Christian König wrote: Am 14.03.23 um 15:18 schrieb Tvrtko Ursulin: From: Tvrtko Ursulin Thread group id (aka pid from userspace point of view) is a more interesting thing to show as an owner of a DRM fd, so track and show that instead of the thread id. In the next patch we will make the owner updated post file descriptor handover, which will also be tgid based to avoid ping-pong when multiple threads access the fd. Signed-off-by: Tvrtko Ursulin Cc: Zack Rusin Cc: linux-graphics-maintai...@vmware.com Cc: Alex Deucher Cc: "Christian König" Reviewed-by: Zack Rusin Reviewed-by: Christian König Should we push the already reviewed cleanups like this one to drm-misc-next? That makes sense even without the rest of the functionality and reduce the amount of patches re-send. I don't have the commit rights so if you could do that I certainly would not mind, thanks! Regards, Tvrtko --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +- drivers/gpu/drm/drm_debugfs.c | 4 ++-- drivers/gpu/drm/drm_file.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index d8e683688daa..863cb668e000 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -969,7 +969,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused) * Therefore, we need to protect this ->comm access using RCU. */ rcu_read_lock(); - task = pid_task(file->pid, PIDTYPE_PID); + task = pid_task(file->pid, PIDTYPE_TGID); seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), task ? task->comm : ""); rcu_read_unlock(); diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 4f643a490dc3..4855230ba2c6 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -80,7 +80,7 @@ static int drm_clients_info(struct seq_file *m, void *data) seq_printf(m, "%20s %5s %3s master a %5s %10s\n", "command", - "pid", + "tgid", "dev", "uid", "magic"); @@ -94,7 +94,7 @@ static int drm_clients_info(struct seq_file *m, void *data) bool is_current_master = drm_is_current_master(priv); rcu_read_lock(); /* locks pid_task()->comm */ - task = pid_task(priv->pid, PIDTYPE_PID); + task = pid_task(priv->pid, PIDTYPE_TGID); uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID; seq_printf(m, "%20s %5d %3d %c %c %5d %10u\n", task ? task->comm : "", diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index a51ff8cee049..c1018c470047 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -156,7 +156,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) if (!file) return ERR_PTR(-ENOMEM); - file->pid = get_pid(task_pid(current)); + file->pid = get_pid(task_tgid(current)); file->minor = minor; /* for compatibility root is always authenticated */ diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index d6baf73a6458..c0da89e16e6f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -241,7 +241,7 @@ static int vmw_debugfs_gem_info_show(struct seq_file *m, void *unused) * Therefore, we need to protect this ->comm access using RCU. */ rcu_read_lock(); - task = pid_task(file->pid, PIDTYPE_PID); + task = pid_task(file->pid, PIDTYPE_TGID); seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), task ? task->comm : ""); rcu_read_unlock();
[PULL] drm-intel-fixes
Hi Dave & Daniel - Missed fixes last week, so here we are early this week. drm-intel-fixes-2023-03-15: drm/i915 fixes for v6.3-rc3: - Fix hwmon PL1 power limit enabling - Fix audio ELD handling for DP MST - Fix PSR io and wake line calculations - Fix DG2 HDMI modes with 267.30 and 319.89 MHz pixel clocks - Fix SSEU subslice out-of-bounds access - Fix misuse of non-idle barriers as fence trackers BR, Jani. The following changes since commit eeac8ede17557680855031c6f305ece2378af326: Linux 6.3-rc2 (2023-03-12 16:36:44 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2023-03-15 for you to fetch changes up to e0e6b416b25ee14716f3549e0cbec1011b193809: drm/i915/active: Fix misuse of non-idle barriers as fence trackers (2023-03-13 11:38:05 +0200) drm/i915 fixes for v6.3-rc3: - Fix hwmon PL1 power limit enabling - Fix audio ELD handling for DP MST - Fix PSR io and wake line calculations - Fix DG2 HDMI modes with 267.30 and 319.89 MHz pixel clocks - Fix SSEU subslice out-of-bounds access - Fix misuse of non-idle barriers as fence trackers Andrea Righi (1): drm/i915/sseu: fix max_subslices array-index-out-of-bounds access Ankit Nautiyal (1): drm/i915/dg2: Add HDMI pixel clock frequencies 267.30 and 319.89 MHz Ashutosh Dixit (1): drm/i915/hwmon: Enable PL1 power limit Janusz Krzysztofik (1): drm/i915/active: Fix misuse of non-idle barriers as fence trackers Jouni Högander (1): drm/i915/psr: Use calculated io and fast wake lines Ville Syrjälä (1): drm/i915: Fix audio ELD handling for DP MST drivers/gpu/drm/i915/display/intel_display_types.h | 2 + drivers/gpu/drm/i915/display/intel_dp_mst.c| 25 --- drivers/gpu/drm/i915/display/intel_psr.c | 78 +- drivers/gpu/drm/i915/display/intel_snps_phy.c | 62 + drivers/gpu/drm/i915/gt/intel_sseu.h | 2 +- drivers/gpu/drm/i915/i915_active.c | 25 --- drivers/gpu/drm/i915/i915_hwmon.c | 5 ++ 7 files changed, 161 insertions(+), 38 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 30/36] drm/i915/huc: use const struct bus_type pointers
On 13/03/2023 18:29, Greg Kroah-Hartman wrote: The struct bus_type pointers in the functions intel_huc_register_gsc_notifier() and intel_huc_unregister_gsc_notifier() should be a const pointer, as the structure is not modified anywhere in the functions, and the pointer they are passed will be a const * in the near future. Cc: Jani Nikula Cc: Joonas Lahtinen Cc: Rodrigo Vivi Cc: Tvrtko Ursulin Cc: David Airlie Cc: Daniel Vetter Cc: Daniele Ceraolo Spurio Cc: Alan Previn Cc: John Harrison Cc: Greg Kroah-Hartman Cc: Tony Ye Cc: Vitaly Lubart Cc: intel-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Greg Kroah-Hartman --- Note, this is a patch that is a prepatory cleanup as part of a larger series of patches that is working on resolving some old driver core design mistakes. It will build and apply cleanly on top of 6.3-rc2 on its own, but I'd prefer if I could take it through my driver-core tree so that the driver core changes can be taken through there for 6.4-rc1. Sounds fine to me. Acked-by: Tvrtko Ursulin Regards, Tvrtko drivers/gpu/drm/i915/gt/uc/intel_huc.c | 4 ++-- drivers/gpu/drm/i915/gt/uc/intel_huc.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c index 410905da8e97..8b453bd7c953 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c @@ -183,7 +183,7 @@ static int gsc_notifier(struct notifier_block *nb, unsigned long action, void *d return 0; } -void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type *bus) +void intel_huc_register_gsc_notifier(struct intel_huc *huc, const struct bus_type *bus) { int ret; @@ -200,7 +200,7 @@ void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type *bus } } -void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type *bus) +void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, const struct bus_type *bus) { if (!huc->delayed_load.nb.notifier_call) return; diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.h b/drivers/gpu/drm/i915/gt/uc/intel_huc.h index 52db03620c60..05d4832f8461 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.h @@ -51,8 +51,8 @@ int intel_huc_check_status(struct intel_huc *huc); void intel_huc_update_auth_status(struct intel_huc *huc); bool intel_huc_is_authenticated(struct intel_huc *huc); -void intel_huc_register_gsc_notifier(struct intel_huc *huc, struct bus_type *bus); -void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type *bus); +void intel_huc_register_gsc_notifier(struct intel_huc *huc, const struct bus_type *bus); +void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, const struct bus_type *bus); static inline int intel_huc_sanitize(struct intel_huc *huc) {
Re: [PATCH][next] habanalabs: Fix spelling mistake "maped" -> "mapped"
On Tue, Mar 14, 2023 at 10:18 AM Colin Ian King wrote: > > There is a spelling mistake in a dev_err message. Fix it. > > Signed-off-by: Colin Ian King > --- > drivers/accel/habanalabs/common/memory_mgr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/accel/habanalabs/common/memory_mgr.c > b/drivers/accel/habanalabs/common/memory_mgr.c > index 30f8059f28c2..c4d84df355b0 100644 > --- a/drivers/accel/habanalabs/common/memory_mgr.c > +++ b/drivers/accel/habanalabs/common/memory_mgr.c > @@ -275,7 +275,7 @@ int hl_mem_mgr_mmap(struct hl_mem_mgr *mmg, struct > vm_area_struct *vma, > > if (atomic_cmpxchg(&buf->mmap, 0, 1)) { > dev_err(mmg->dev, > - "%s, Memory mmap failed, already maped to user\n", > + "%s, Memory mmap failed, already mapped to user\n", > buf->behavior->topic); > rc = -EINVAL; > goto put_mem; > -- > 2.30.2 > Reviewed-by: Oded Gabbay Applied to -next Thanks, Oded
Re: [PATCH v3 35/38] video: handle HAS_IOPORT dependencies
On Wed, Mar 15, 2023 at 09:16:50AM +0100, Geert Uytterhoeven wrote: > Hi Niklas, > > On Tue, Mar 14, 2023 at 1:13 PM Niklas Schnelle > wrote: > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > > not being declared. We thus need to add HAS_IOPORT as dependency for > > those drivers using them and guard inline code in headers. > > > > Co-developed-by: Arnd Bergmann > > Signed-off-by: Niklas Schnelle > > Thanks for your patch! > > > --- a/drivers/video/fbdev/Kconfig > > +++ b/drivers/video/fbdev/Kconfig > > > @@ -1284,7 +1285,7 @@ config FB_ATY128_BACKLIGHT > > > > config FB_ATY > > tristate "ATI Mach64 display support" if PCI || ATARI > > - depends on FB && !SPARC32 > > + depends on FB && HAS_IOPORT && !SPARC32 > > On Atari, this works without ATARI_ROM_ISA, hence it must not depend > on HAS_IOPORT. > The only call to inb() is inside a section protected by #ifdef > CONFIG_PCI. So: That piece of code is a nop anyway. We immediately overwrite clk_wr_offset with a hardcoded selection after the register reads. So if you nuke that nop code then no IOPORT dependency required at all. > > depends on FB && !SPARC32 > depends on ATARI || HAS_IOPORT > > > select FB_CFB_FILLRECT > > select FB_CFB_COPYAREA > > select FB_CFB_IMAGEBLIT > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds -- Ville Syrjälä Intel
Re: [PATCH] fbdev: au1200fb: Fix potential divide by zero
Hi, thanks for looking through these drivers. Am 15.03.23 um 10:22 schrieb Wei Chen: var->pixclock can be assigned to zero by user. Without proper check, divide by zero would occur when invoking macro PICOS2KHZ in au1200fb_fb_check_var. Error out if var->pixclock is zero. Signed-off-by: Wei Chen --- drivers/video/fbdev/au1200fb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c index 81c315454428..b6b22fa4a8a0 100644 --- a/drivers/video/fbdev/au1200fb.c +++ b/drivers/video/fbdev/au1200fb.c @@ -1040,6 +1040,9 @@ static int au1200fb_fb_check_var(struct fb_var_screeninfo *var, u32 pixclock; int screen_size, plane; + if (!var->pixclock) + return -EINVAL; + Instead of the whale-a-mole approach of fixing individual drivers, could this be solved by testing in fb_set_var [1] and fb_try_mode.? [2] Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbmem.c#L958 [2] https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/modedb.c#L567 plane = fbdev->plane; /* Make sure that the mode respect all LCD controller and -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[PATCH] drm/i915/gem: Clarify seemingly unaccounted obj refcount inc
Add a comment why there is a obj refcount inc before installing the vm_ops for the mmap call. Also remove the invalid older comment as drm API(drm_gem_prime_mmap()) will hold an obj reference before calling this driver mmap callback so we can't have 0-refcnted object here. Cc: Matthew Auld Cc: Andi Shyti Signed-off-by: Nirmoy Das --- drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index d3c1dee16af2..0bc8c3818443 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -952,9 +952,10 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma) vma_pages(vma)); if (node && drm_vma_node_is_allowed(node, priv)) { /* -* Skip 0-refcnted objects as it is in the process of being -* destroyed and will be invalid when the vma manager lock -* is released. +* When we install vm_ops for mmap we are too late for +* the vm_ops->open() which increases the ref_count of +* this obj and then it gets decreased by the vm_ops->close(). +* To balance this increase the obj ref_count here. */ if (!node->driver_private) { mmo = container_of(node, struct i915_mmap_offset, vma_node); -- 2.39.0
Re: [PATCH] drm/ttm: Fix excess doc paramater warning in ttm_bo_mem_space
On 3/14/23 01:20, Yussef Fatayer wrote: Rename documentation parameter from "proposed_placement" to "placement" to avoid the warnings of function parameter not described and excess function parameter. Signed-off-by: Yussef Fatayer Reviewed-by: Maíra Canal Best Regards, - Maíra Canal --- drivers/gpu/drm/ttm/ttm_bo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 459f1b4440da..8284f4d0ab21 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -748,7 +748,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, * * @bo: Pointer to a struct ttm_buffer_object. the data of which * we want to allocate space for. - * @proposed_placement: Proposed new placement for the buffer object. + * @placement: Proposed new placement for the buffer object. * @mem: A struct ttm_resource. * @ctx: if and how to sleep, lock buffers and alloc memory *
Re: [PATCH] drm/komeda: Take over EFI framebuffer properly
On Wed, Mar 15, 2023 at 09:34:37AM +, Steven Price wrote: > On 13/03/2023 10:22, patrik.bergl...@arm.com wrote: > > From: Patrik Berglund > > > > The Arm Morello board EDK2 port already provides an EFI GOP display for > > Ceti/Cetus (Komeda) with more boards incoming. > > However, once the Komeda driver probes and takes over the hardware, > > it should take over the logical framebuffer as well, otherwise, > > the now-defunct GOP device hangs around and virtual console output > > inevitably disappears into the wrong place most of the time. > > > > We'll do this right before doing the SRST because that is the point > > when the GOP will stop working. > > The GOP might also fail because the encoder driver do things but this > > is better than nothing. > > > > Signed-off-by: Patrik Berglund > > +CC the maintainers. > > Looks right to me, hdlcd has something very similar. > > Reviewed-by: Steven Price Hi Steven, Patrik contacted me privately and made me aware of the patch. I had a discussion with him about the layering violation (d71 knowing about kms) and we came to the conclusion that an additional patch is needed to re-order some code before this one can go in. Patrik and/or I need to revive a test environment to check that patch. Best regards, Liviu > > > --- > > drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 12 > > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 6 ++ > > drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 1 + > > 3 files changed, 19 insertions(+) > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c > > b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c > > index 6c56f5662bc7..72035af9bc5f 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c > > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c > > @@ -8,6 +8,7 @@ > > #include > > #include > > #include "d71_dev.h" > > +#include "komeda_kms.h" > > #include "malidp_io.h" > > > > static u64 get_lpu_event(struct d71_pipeline *d71_pipeline) > > @@ -310,6 +311,17 @@ static int d71_reset(struct d71_dev *d71) > > u32 __iomem *gcu = d71->gcu_addr; > > int ret; > > > > + /* > > +* If we are already running, the most likely reason is that the EFI > > left > > +* us running (GOP), so make sure to take over from simple framebuffer > > +* drivers. > > +*/ > > + if (malidp_read32(gcu, BLK_STATUS) & GCU_STATUS_ACTIVE) { > > + ret = komeda_kms_remove_framebuffers(); > > + if (ret) > > + return ret; > > + } > > + > > malidp_write32(gcu, BLK_CONTROL, GCU_CONTROL_SRST); > > > > ret = dp_wait_cond(!(malidp_read32(gcu, BLK_CONTROL) & > > GCU_CONTROL_SRST), > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > > index 62dc64550793..12af409aeabb 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > > @@ -7,6 +7,7 @@ > > #include > > #include > > > > +#include > > #include > > #include > > #include > > @@ -349,3 +350,8 @@ void komeda_kms_detach(struct komeda_kms_dev *kms) > > komeda_kms_cleanup_private_objs(kms); > > drm->dev_private = NULL; > > } > > + > > +int komeda_kms_remove_framebuffers(void) > > +{ > > + return drm_aperture_remove_framebuffers(false, &komeda_kms_driver); > > +} > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h > > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h > > index 3a872c292091..1a43707ed68f 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h > > @@ -187,5 +187,6 @@ void komeda_crtc_flush_and_wait_for_flip_done(struct > > komeda_crtc *kcrtc, > > > > struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev); > > void komeda_kms_detach(struct komeda_kms_dev *kms); > > +int komeda_kms_remove_framebuffers(void); > > > > #endif /*_KOMEDA_KMS_H_*/ > -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯
Re: [PATCH] drm/komeda: Take over EFI framebuffer properly
On 15/03/2023 11:10, Liviu Dudau wrote: > On Wed, Mar 15, 2023 at 09:34:37AM +, Steven Price wrote: >> On 13/03/2023 10:22, patrik.bergl...@arm.com wrote: >>> From: Patrik Berglund >>> >>> The Arm Morello board EDK2 port already provides an EFI GOP display for >>> Ceti/Cetus (Komeda) with more boards incoming. >>> However, once the Komeda driver probes and takes over the hardware, >>> it should take over the logical framebuffer as well, otherwise, >>> the now-defunct GOP device hangs around and virtual console output >>> inevitably disappears into the wrong place most of the time. >>> >>> We'll do this right before doing the SRST because that is the point >>> when the GOP will stop working. >>> The GOP might also fail because the encoder driver do things but this >>> is better than nothing. >>> >>> Signed-off-by: Patrik Berglund >> >> +CC the maintainers. >> >> Looks right to me, hdlcd has something very similar. >> >> Reviewed-by: Steven Price > > Hi Steven, > > Patrik contacted me privately and made me aware of the patch. I had a > discussion > with him about the layering violation (d71 knowing about kms) and we came to > the > conclusion that an additional patch is needed to re-order some code before > this > one can go in. Patrik and/or I need to revive a test environment to check that > patch. Ah right, fair enough. Patrik pinged me privately too to look at this and so I did a brief review and this looks like it solves a real issue. But I agree if this can be solved without d71 knowing about kms that would be much better. It's good to know the conversation is happening! Thanks, Steve > Best regards, > Liviu > >> >>> --- >>> drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 12 >>> drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 6 ++ >>> drivers/gpu/drm/arm/display/komeda/komeda_kms.h | 1 + >>> 3 files changed, 19 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c >>> b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c >>> index 6c56f5662bc7..72035af9bc5f 100644 >>> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c >>> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c >>> @@ -8,6 +8,7 @@ >>> #include >>> #include >>> #include "d71_dev.h" >>> +#include "komeda_kms.h" >>> #include "malidp_io.h" >>> >>> static u64 get_lpu_event(struct d71_pipeline *d71_pipeline) >>> @@ -310,6 +311,17 @@ static int d71_reset(struct d71_dev *d71) >>> u32 __iomem *gcu = d71->gcu_addr; >>> int ret; >>> >>> + /* >>> +* If we are already running, the most likely reason is that the EFI >>> left >>> +* us running (GOP), so make sure to take over from simple framebuffer >>> +* drivers. >>> +*/ >>> + if (malidp_read32(gcu, BLK_STATUS) & GCU_STATUS_ACTIVE) { >>> + ret = komeda_kms_remove_framebuffers(); >>> + if (ret) >>> + return ret; >>> + } >>> + >>> malidp_write32(gcu, BLK_CONTROL, GCU_CONTROL_SRST); >>> >>> ret = dp_wait_cond(!(malidp_read32(gcu, BLK_CONTROL) & >>> GCU_CONTROL_SRST), >>> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c >>> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c >>> index 62dc64550793..12af409aeabb 100644 >>> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c >>> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c >>> @@ -7,6 +7,7 @@ >>> #include >>> #include >>> >>> +#include >>> #include >>> #include >>> #include >>> @@ -349,3 +350,8 @@ void komeda_kms_detach(struct komeda_kms_dev *kms) >>> komeda_kms_cleanup_private_objs(kms); >>> drm->dev_private = NULL; >>> } >>> + >>> +int komeda_kms_remove_framebuffers(void) >>> +{ >>> + return drm_aperture_remove_framebuffers(false, &komeda_kms_driver); >>> +} >>> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h >>> b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h >>> index 3a872c292091..1a43707ed68f 100644 >>> --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h >>> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h >>> @@ -187,5 +187,6 @@ void komeda_crtc_flush_and_wait_for_flip_done(struct >>> komeda_crtc *kcrtc, >>> >>> struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev); >>> void komeda_kms_detach(struct komeda_kms_dev *kms); >>> +int komeda_kms_remove_framebuffers(void); >>> >>> #endif /*_KOMEDA_KMS_H_*/ >> >
Re: [PATCH] drm/i915/gem: Clarify seemingly unaccounted obj refcount inc
Hi Nirmoy, On Wed, Mar 15, 2023 at 11:54:46AM +0100, Nirmoy Das wrote: > Add a comment why there is a obj refcount inc before installing > the vm_ops for the mmap call. Also remove the invalid older comment > as drm API(drm_gem_prime_mmap()) will hold an obj reference before > calling this driver mmap callback so we can't have 0-refcnted > object here. > > Cc: Matthew Auld > Cc: Andi Shyti > Signed-off-by: Nirmoy Das fine with me! Reviewed-by: Andi Shyti Andi > --- > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > index d3c1dee16af2..0bc8c3818443 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > @@ -952,9 +952,10 @@ int i915_gem_mmap(struct file *filp, struct > vm_area_struct *vma) > vma_pages(vma)); > if (node && drm_vma_node_is_allowed(node, priv)) { > /* > - * Skip 0-refcnted objects as it is in the process of being > - * destroyed and will be invalid when the vma manager lock > - * is released. > + * When we install vm_ops for mmap we are too late for > + * the vm_ops->open() which increases the ref_count of > + * this obj and then it gets decreased by the vm_ops->close(). > + * To balance this increase the obj ref_count here. >*/ > if (!node->driver_private) { > mmo = container_of(node, struct i915_mmap_offset, > vma_node); > -- > 2.39.0
Re: [PATCH v3 07/38] drm: handle HAS_IOPORT dependencies
On Tue, 14 Mar 2023, Niklas Schnelle wrote: > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > not being declared. We thus need to add HAS_IOPORT as dependency for > those drivers using them. In the bochs driver there is optional MMIO > support detected at runtime, warn if this isn't taken when > HAS_IOPORT is not defined. Not that I care a whole lot, but there should really only be one warning or even failure to probe at bochs_hw_init() for !bochs->mmio && !IS_ENABLED(CONFIG_HAS_IOPORT), not warnings all over the place. Moreover, the config macro is CONFIG_HAS_IOPORT instead of HAS_IOPORT that you check for below. BR, Jani. > There is also a direct and hard coded use in cirrus.c which according to > the comment is only necessary during resume. Let's just skip this as > for example s390 which doesn't have I/O port support also doesen't > support suspend/resume. > > Co-developed-by: Arnd Bergmann > Signed-off-by: Niklas Schnelle > --- > drivers/gpu/drm/qxl/Kconfig | 1 + > drivers/gpu/drm/tiny/bochs.c | 19 +++ > drivers/gpu/drm/tiny/cirrus.c | 2 ++ > 3 files changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig > index ca3f51c2a8fe..d0e0d440c8d9 100644 > --- a/drivers/gpu/drm/qxl/Kconfig > +++ b/drivers/gpu/drm/qxl/Kconfig > @@ -2,6 +2,7 @@ > config DRM_QXL > tristate "QXL virtual GPU" > depends on DRM && PCI && MMU > + depends on HAS_IOPORT > select DRM_KMS_HELPER > select DRM_TTM > select DRM_TTM_HELPER > diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c > index 024346054c70..da4a5d53b003 100644 > --- a/drivers/gpu/drm/tiny/bochs.c > +++ b/drivers/gpu/drm/tiny/bochs.c > @@ -2,6 +2,7 @@ > > #include > #include > +#include > > #include > #include > @@ -105,7 +106,11 @@ static void bochs_vga_writeb(struct bochs_device *bochs, > u16 ioport, u8 val) > > writeb(val, bochs->mmio + offset); > } else { > +#ifdef HAS_IOPORT > outb(val, ioport); > +#else > + WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT"); > +#endif > } > } > > @@ -119,7 +124,12 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, > u16 ioport) > > return readb(bochs->mmio + offset); > } else { > +#ifdef HAS_IOPORT > return inb(ioport); > +#else > + WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT"); > + return 0xff; > +#endif > } > } > > @@ -132,8 +142,13 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, > u16 reg) > > ret = readw(bochs->mmio + offset); > } else { > +#ifdef HAS_IOPORT > outw(reg, VBE_DISPI_IOPORT_INDEX); > ret = inw(VBE_DISPI_IOPORT_DATA); > +#else > + WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT"); > + ret = 0x; > +#endif > } > return ret; > } > @@ -145,8 +160,12 @@ static void bochs_dispi_write(struct bochs_device > *bochs, u16 reg, u16 val) > > writew(val, bochs->mmio + offset); > } else { > +#ifdef HAS_IOPORT > outw(reg, VBE_DISPI_IOPORT_INDEX); > outw(val, VBE_DISPI_IOPORT_DATA); > +#else > + WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT"); > +#endif > } > } > > diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c > index accfa52e78c5..9da89732c5ac 100644 > --- a/drivers/gpu/drm/tiny/cirrus.c > +++ b/drivers/gpu/drm/tiny/cirrus.c > @@ -308,8 +308,10 @@ static int cirrus_mode_set(struct cirrus_device *cirrus, > > cirrus_set_start_address(cirrus, 0); > > +#ifdef CONFIG_HAS_IOPORT > /* Unblank (needed on S3 resume, vgabios doesn't do it then) */ > outb(0x20, 0x3c0); > +#endif > > drm_dev_exit(idx); > return 0; -- Jani Nikula, Intel Open Source Graphics Center
[PATCH v2] drm/amdgpu/nv: Apply ASPM quirk on Intel ADL + AMD Navi
S2idle resume freeze can be observed on Intel ADL + AMD WX5500. This is caused by commit 0064b0ce85bb ("drm/amd/pm: enable ASPM by default"). The root cause is still not clear for now. So extend and apply the ASPM quirk from commit e02fe3bc7aba ("drm/amdgpu: vi: disable ASPM on Intel Alder Lake based systems"), to workaround the issue on Navi cards too. Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default") Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2458 Reviewed-by: Alex Deucher Signed-off-by: Kai-Heng Feng --- v2: - Rename the quirk function. drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 +++ drivers/gpu/drm/amd/amdgpu/nv.c| 2 +- drivers/gpu/drm/amd/amdgpu/vi.c| 17 + 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 164141bc8b4a..5f3b139c1f99 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1272,6 +1272,7 @@ void amdgpu_device_pci_config_reset(struct amdgpu_device *adev); int amdgpu_device_pci_reset(struct amdgpu_device *adev); bool amdgpu_device_need_post(struct amdgpu_device *adev); bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev); +bool amdgpu_device_aspm_support_quirk(void); void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes, u64 num_vis_bytes); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index c4a4e2fe6681..05a34ff79e78 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -80,6 +80,10 @@ #include +#if IS_ENABLED(CONFIG_X86) +#include +#endif + MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin"); MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin"); MODULE_FIRMWARE("amdgpu/raven_gpu_info.bin"); @@ -1356,6 +1360,17 @@ bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev) return pcie_aspm_enabled(adev->pdev); } +bool amdgpu_device_aspm_support_quirk(void) +{ +#if IS_ENABLED(CONFIG_X86) + struct cpuinfo_x86 *c = &cpu_data(0); + + return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE); +#else + return true; +#endif +} + /* if we get transitioned to only one device, take VGA back */ /** * amdgpu_device_vga_set_decode - enable/disable vga decode diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c index 855d390c41de..26733263913e 100644 --- a/drivers/gpu/drm/amd/amdgpu/nv.c +++ b/drivers/gpu/drm/amd/amdgpu/nv.c @@ -578,7 +578,7 @@ static void nv_pcie_gen3_enable(struct amdgpu_device *adev) static void nv_program_aspm(struct amdgpu_device *adev) { - if (!amdgpu_device_should_use_aspm(adev)) + if (!amdgpu_device_should_use_aspm(adev) || !amdgpu_device_aspm_support_quirk()) return; if (!(adev->flags & AMD_IS_APU) && diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c index 12ef782eb478..ceab8783575c 100644 --- a/drivers/gpu/drm/amd/amdgpu/vi.c +++ b/drivers/gpu/drm/amd/amdgpu/vi.c @@ -81,10 +81,6 @@ #include "mxgpu_vi.h" #include "amdgpu_dm.h" -#if IS_ENABLED(CONFIG_X86) -#include -#endif - #define ixPCIE_LC_L1_PM_SUBSTATE 0x100100C6 #define PCIE_LC_L1_PM_SUBSTATE__LC_L1_SUBSTATES_OVERRIDE_EN_MASK 0x0001L #define PCIE_LC_L1_PM_SUBSTATE__LC_PCI_PM_L1_2_OVERRIDE_MASK 0x0002L @@ -1138,24 +1134,13 @@ static void vi_enable_aspm(struct amdgpu_device *adev) WREG32_PCIE(ixPCIE_LC_CNTL, data); } -static bool aspm_support_quirk_check(void) -{ -#if IS_ENABLED(CONFIG_X86) - struct cpuinfo_x86 *c = &cpu_data(0); - - return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE); -#else - return true; -#endif -} - static void vi_program_aspm(struct amdgpu_device *adev) { u32 data, data1, orig; bool bL1SS = false; bool bClkReqSupport = true; - if (!amdgpu_device_should_use_aspm(adev) || !aspm_support_quirk_check()) + if (!amdgpu_device_should_use_aspm(adev) || !amdgpu_device_aspm_support_quirk()) return; if (adev->flags & AMD_IS_APU || -- 2.34.1
[PATCH 5.4 03/68] drm/connector: print max_requested_bpc in state debugfs
From: Harry Wentland commit 7d386975f6a495902e679a3a250a7456d7e54765 upstream. This is useful to understand the bpc defaults and support of a driver. Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Uma Shankar Cc: Ville Syrjälä Cc: Joshua Ashton Cc: Jani Nikula Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Reviewed-By: Joshua Ashton Link: https://patchwork.freedesktop.org/patch/msgid/20230113162428.33874-3-harry.wentl...@amd.com Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/drm_atomic.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1006,6 +1006,7 @@ static void drm_atomic_connector_print_s drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name); drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); drm_printf(p, "\tself_refresh_aware=%d\n", state->self_refresh_aware); + drm_printf(p, "\tmax_requested_bpc=%d\n", state->max_requested_bpc); if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) if (state->writeback_job && state->writeback_job->fb)
[PATCH 5.10 029/104] drm/nouveau/kms/nv50: fix nv50_wndw_new_ prototype
From: Jiri Slaby (SUSE) [ Upstream commit 3638a820c5c3b52f327cebb174fd4274bee08aa7 ] gcc-13 warns about mismatching types for enums. That revealed switched arguments of nv50_wndw_new_(): drivers/gpu/drm/nouveau/dispnv50/wndw.c:696:1: error: conflicting types for 'nv50_wndw_new_' due to enum/integer mismatch; have 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, u32, enum nv50_disp_interlock_type, u32, struct nv50_wndw **)' drivers/gpu/drm/nouveau/dispnv50/wndw.h:36:5: note: previous declaration of 'nv50_wndw_new_' with type 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, enum nv50_disp_interlock_type, u32, u32, struct nv50_wndw **)' It can be barely visible, but the declaration says about the parameters in the middle: enum nv50_disp_interlock_type, u32 interlock_data, u32 heads, While the definition states differently: u32 heads, enum nv50_disp_interlock_type interlock_type, u32 interlock_data, Unify/fix the declaration to match the definition. Fixes: 53e0a3e70de6 ("drm/nouveau/kms/nv50-: simplify tracking of channel interlocks") Cc: Martin Liska Cc: Ben Skeggs Cc: Karol Herbst Cc: Lyude Paul Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Jiri Slaby (SUSE) Signed-off-by: Karol Herbst Link: https://patchwork.freedesktop.org/patch/msgid/20221031114229.10289-1-jirisl...@kernel.org Signed-off-by: Sasha Levin --- drivers/gpu/drm/nouveau/dispnv50/wndw.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h index 8bed195ae098a..77bf124319fbd 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h @@ -38,8 +38,9 @@ struct nv50_wndw { int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *name, int index, - const u32 *format, enum nv50_disp_interlock_type, - u32 interlock_data, u32 heads, struct nv50_wndw **); + const u32 *format, u32 heads, + enum nv50_disp_interlock_type, u32 interlock_data, + struct nv50_wndw **); void nv50_wndw_flush_set(struct nv50_wndw *, u32 *interlock, struct nv50_wndw_atom *); void nv50_wndw_flush_clr(struct nv50_wndw *, u32 *interlock, bool flush, -- 2.39.2
[PATCH 5.10 005/104] drm/connector: print max_requested_bpc in state debugfs
From: Harry Wentland commit 7d386975f6a495902e679a3a250a7456d7e54765 upstream. This is useful to understand the bpc defaults and support of a driver. Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Uma Shankar Cc: Ville Syrjälä Cc: Joshua Ashton Cc: Jani Nikula Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Reviewed-By: Joshua Ashton Link: https://patchwork.freedesktop.org/patch/msgid/20230113162428.33874-3-harry.wentl...@amd.com Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/drm_atomic.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1010,6 +1010,7 @@ static void drm_atomic_connector_print_s drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name); drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); drm_printf(p, "\tself_refresh_aware=%d\n", state->self_refresh_aware); + drm_printf(p, "\tmax_requested_bpc=%d\n", state->max_requested_bpc); if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) if (state->writeback_job && state->writeback_job->fb)
[PATCH 5.15 007/145] drm/connector: print max_requested_bpc in state debugfs
From: Harry Wentland commit 7d386975f6a495902e679a3a250a7456d7e54765 upstream. This is useful to understand the bpc defaults and support of a driver. Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Uma Shankar Cc: Ville Syrjälä Cc: Joshua Ashton Cc: Jani Nikula Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Reviewed-By: Joshua Ashton Link: https://patchwork.freedesktop.org/patch/msgid/20230113162428.33874-3-harry.wentl...@amd.com Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/drm_atomic.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1052,6 +1052,7 @@ static void drm_atomic_connector_print_s drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name); drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); drm_printf(p, "\tself_refresh_aware=%d\n", state->self_refresh_aware); + drm_printf(p, "\tmax_requested_bpc=%d\n", state->max_requested_bpc); if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) if (state->writeback_job && state->writeback_job->fb)
[PATCH 5.15 044/145] drm/nouveau/kms/nv50: fix nv50_wndw_new_ prototype
From: Jiri Slaby (SUSE) [ Upstream commit 3638a820c5c3b52f327cebb174fd4274bee08aa7 ] gcc-13 warns about mismatching types for enums. That revealed switched arguments of nv50_wndw_new_(): drivers/gpu/drm/nouveau/dispnv50/wndw.c:696:1: error: conflicting types for 'nv50_wndw_new_' due to enum/integer mismatch; have 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, u32, enum nv50_disp_interlock_type, u32, struct nv50_wndw **)' drivers/gpu/drm/nouveau/dispnv50/wndw.h:36:5: note: previous declaration of 'nv50_wndw_new_' with type 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, enum nv50_disp_interlock_type, u32, u32, struct nv50_wndw **)' It can be barely visible, but the declaration says about the parameters in the middle: enum nv50_disp_interlock_type, u32 interlock_data, u32 heads, While the definition states differently: u32 heads, enum nv50_disp_interlock_type interlock_type, u32 interlock_data, Unify/fix the declaration to match the definition. Fixes: 53e0a3e70de6 ("drm/nouveau/kms/nv50-: simplify tracking of channel interlocks") Cc: Martin Liska Cc: Ben Skeggs Cc: Karol Herbst Cc: Lyude Paul Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Jiri Slaby (SUSE) Signed-off-by: Karol Herbst Link: https://patchwork.freedesktop.org/patch/msgid/20221031114229.10289-1-jirisl...@kernel.org Signed-off-by: Sasha Levin --- drivers/gpu/drm/nouveau/dispnv50/wndw.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h index 980f8ea96d54a..6c64864da4550 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h @@ -38,8 +38,9 @@ struct nv50_wndw { int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *name, int index, - const u32 *format, enum nv50_disp_interlock_type, - u32 interlock_data, u32 heads, struct nv50_wndw **); + const u32 *format, u32 heads, + enum nv50_disp_interlock_type, u32 interlock_data, + struct nv50_wndw **); void nv50_wndw_flush_set(struct nv50_wndw *, u32 *interlock, struct nv50_wndw_atom *); void nv50_wndw_flush_clr(struct nv50_wndw *, u32 *interlock, bool flush, -- 2.39.2
Re: [PATCH] fbdev: au1200fb: Fix potential divide by zero
Dear Thomas, Thank you for the kind advice. In fact, I notice there was a discussion regarding whether to put the check in each individual driver or solve this problem as a whole as you suggested. The conclusion is that it is better to keep the check per driver rather than in the caller. Related discussions are here: https://lore.kernel.org/all/yxclzqgftr1nf...@ravnborg.org/ https://lore.kernel.org/all/ypgbhmtlqqb1k...@ravnborg.org/ https://lore.kernel.org/all/20220404084723.79089-1-zheyum...@gmail.com/ Thanks, Wei -- Original Message -- >From "Thomas Zimmermann" To "Wei Chen" ; del...@gmx.de Cc linux-fb...@vger.kernel.org; linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org Date 2023/3/15 18:25:52 Subject Re: [PATCH] fbdev: au1200fb: Fix potential divide by zero On Wed, 15 Mar 2023 at 18:25, Thomas Zimmermann wrote: > > Hi, > > thanks for looking through these drivers. > > Am 15.03.23 um 10:22 schrieb Wei Chen: > > var->pixclock can be assigned to zero by user. Without > > proper check, divide by zero would occur when invoking > > macro PICOS2KHZ in au1200fb_fb_check_var. > > > > Error out if var->pixclock is zero. > > > > Signed-off-by: Wei Chen > > --- > > drivers/video/fbdev/au1200fb.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c > > index 81c315454428..b6b22fa4a8a0 100644 > > --- a/drivers/video/fbdev/au1200fb.c > > +++ b/drivers/video/fbdev/au1200fb.c > > @@ -1040,6 +1040,9 @@ static int au1200fb_fb_check_var(struct > > fb_var_screeninfo *var, > > u32 pixclock; > > int screen_size, plane; > > > > + if (!var->pixclock) > > + return -EINVAL; > > + > > Instead of the whale-a-mole approach of fixing individual drivers, could > this be solved by testing in fb_set_var [1] and fb_try_mode.? [2] > > Best regards > Thomas > > [1] > https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fbmem.c#L958 > [2] > https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/modedb.c#L567 > > > plane = fbdev->plane; > > > > /* Make sure that the mode respect all LCD controller and > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev
[PATCH 6.1 013/143] drm/display: Dont block HDR_OUTPUT_METADATA on unknown EOTF
From: Harry Wentland commit e5eef23e267c72521d81f23f7f82d1f523d4a253 upstream. The EDID of an HDR display defines EOTFs that are supported by the display and can be set in the HDR metadata infoframe. Userspace is expected to read the EDID and set an appropriate HDR_OUTPUT_METADATA. In drm_parse_hdr_metadata_block the kernel reads the supported EOTFs from the EDID and stores them in the drm_connector->hdr_sink_metadata. While doing so it also filters the EOTFs to the EOTFs the kernel knows about. When an HDR_OUTPUT_METADATA is set it then checks to make sure the EOTF is a supported EOTF. In cases where the kernel doesn't know about a new EOTF this check will fail, even if the EDID advertises support. Since it is expected that userspace reads the EDID to understand what the display supports it doesn't make sense for DRM to block an HDR_OUTPUT_METADATA if it contains an EOTF the kernel doesn't understand. This comes with the added benefit of future-proofing metadata support. If the spec defines a new EOTF there is no need to update DRM and an compositor can immediately make use of it. Bug: https://gitlab.freedesktop.org/wayland/weston/-/issues/609 v2: Distinguish EOTFs defind in kernel and ones defined in EDID in the commit description (Pekka) v3: Rebase; drm_hdmi_infoframe_set_hdr_metadata moved to drm_hdmi_helper.c Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Uma Shankar Cc: Ville Syrjälä Cc: Joshua Ashton Cc: Jani Nikula Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Acked-by: Pekka Paalanen Reviewed-By: Joshua Ashton Link: https://patchwork.freedesktop.org/patch/msgid/20230113162428.33874-2-harry.wentl...@amd.com Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/display/drm_hdmi_helper.c |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) --- a/drivers/gpu/drm/display/drm_hdmi_helper.c +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c @@ -44,10 +44,8 @@ int drm_hdmi_infoframe_set_hdr_metadata( /* Sink EOTF is Bit map while infoframe is absolute values */ if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf, - connector->hdr_sink_metadata.hdmi_type1.eotf)) { - DRM_DEBUG_KMS("EOTF Not Supported\n"); - return -EINVAL; - } + connector->hdr_sink_metadata.hdmi_type1.eotf)) + DRM_DEBUG_KMS("Unknown EOTF %d\n", hdr_metadata->hdmi_metadata_type1.eotf); err = hdmi_drm_infoframe_init(frame); if (err < 0)
[PATCH 6.1 014/143] drm/connector: print max_requested_bpc in state debugfs
From: Harry Wentland commit 7d386975f6a495902e679a3a250a7456d7e54765 upstream. This is useful to understand the bpc defaults and support of a driver. Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Uma Shankar Cc: Ville Syrjälä Cc: Joshua Ashton Cc: Jani Nikula Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Reviewed-By: Joshua Ashton Link: https://patchwork.freedesktop.org/patch/msgid/20230113162428.33874-3-harry.wentl...@amd.com Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/drm_atomic.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1070,6 +1070,7 @@ static void drm_atomic_connector_print_s drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name); drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); drm_printf(p, "\tself_refresh_aware=%d\n", state->self_refresh_aware); + drm_printf(p, "\tmax_requested_bpc=%d\n", state->max_requested_bpc); if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) if (state->writeback_job && state->writeback_job->fb)
[PATCH 6.1 058/143] drm/nouveau/kms/nv50: fix nv50_wndw_new_ prototype
From: Jiri Slaby (SUSE) [ Upstream commit 3638a820c5c3b52f327cebb174fd4274bee08aa7 ] gcc-13 warns about mismatching types for enums. That revealed switched arguments of nv50_wndw_new_(): drivers/gpu/drm/nouveau/dispnv50/wndw.c:696:1: error: conflicting types for 'nv50_wndw_new_' due to enum/integer mismatch; have 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, u32, enum nv50_disp_interlock_type, u32, struct nv50_wndw **)' drivers/gpu/drm/nouveau/dispnv50/wndw.h:36:5: note: previous declaration of 'nv50_wndw_new_' with type 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, enum nv50_disp_interlock_type, u32, u32, struct nv50_wndw **)' It can be barely visible, but the declaration says about the parameters in the middle: enum nv50_disp_interlock_type, u32 interlock_data, u32 heads, While the definition states differently: u32 heads, enum nv50_disp_interlock_type interlock_type, u32 interlock_data, Unify/fix the declaration to match the definition. Fixes: 53e0a3e70de6 ("drm/nouveau/kms/nv50-: simplify tracking of channel interlocks") Cc: Martin Liska Cc: Ben Skeggs Cc: Karol Herbst Cc: Lyude Paul Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Jiri Slaby (SUSE) Signed-off-by: Karol Herbst Link: https://patchwork.freedesktop.org/patch/msgid/20221031114229.10289-1-jirisl...@kernel.org Signed-off-by: Sasha Levin --- drivers/gpu/drm/nouveau/dispnv50/wndw.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h index 591c852f326b9..76a6ae5d56526 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h @@ -35,8 +35,9 @@ struct nv50_wndw { int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *name, int index, - const u32 *format, enum nv50_disp_interlock_type, - u32 interlock_data, u32 heads, struct nv50_wndw **); + const u32 *format, u32 heads, + enum nv50_disp_interlock_type, u32 interlock_data, + struct nv50_wndw **); void nv50_wndw_flush_set(struct nv50_wndw *, u32 *interlock, struct nv50_wndw_atom *); void nv50_wndw_flush_clr(struct nv50_wndw *, u32 *interlock, bool flush, -- 2.39.2
[PATCH 6.2 016/141] drm/connector: print max_requested_bpc in state debugfs
From: Harry Wentland commit 7d386975f6a495902e679a3a250a7456d7e54765 upstream. This is useful to understand the bpc defaults and support of a driver. Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Uma Shankar Cc: Ville Syrjälä Cc: Joshua Ashton Cc: Jani Nikula Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Reviewed-By: Joshua Ashton Link: https://patchwork.freedesktop.org/patch/msgid/20230113162428.33874-3-harry.wentl...@amd.com Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/drm_atomic.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1070,6 +1070,7 @@ static void drm_atomic_connector_print_s drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector->name); drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name : "(null)"); drm_printf(p, "\tself_refresh_aware=%d\n", state->self_refresh_aware); + drm_printf(p, "\tmax_requested_bpc=%d\n", state->max_requested_bpc); if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) if (state->writeback_job && state->writeback_job->fb)
[PATCH 6.2 015/141] drm/display: Dont block HDR_OUTPUT_METADATA on unknown EOTF
From: Harry Wentland commit e5eef23e267c72521d81f23f7f82d1f523d4a253 upstream. The EDID of an HDR display defines EOTFs that are supported by the display and can be set in the HDR metadata infoframe. Userspace is expected to read the EDID and set an appropriate HDR_OUTPUT_METADATA. In drm_parse_hdr_metadata_block the kernel reads the supported EOTFs from the EDID and stores them in the drm_connector->hdr_sink_metadata. While doing so it also filters the EOTFs to the EOTFs the kernel knows about. When an HDR_OUTPUT_METADATA is set it then checks to make sure the EOTF is a supported EOTF. In cases where the kernel doesn't know about a new EOTF this check will fail, even if the EDID advertises support. Since it is expected that userspace reads the EDID to understand what the display supports it doesn't make sense for DRM to block an HDR_OUTPUT_METADATA if it contains an EOTF the kernel doesn't understand. This comes with the added benefit of future-proofing metadata support. If the spec defines a new EOTF there is no need to update DRM and an compositor can immediately make use of it. Bug: https://gitlab.freedesktop.org/wayland/weston/-/issues/609 v2: Distinguish EOTFs defind in kernel and ones defined in EDID in the commit description (Pekka) v3: Rebase; drm_hdmi_infoframe_set_hdr_metadata moved to drm_hdmi_helper.c Signed-off-by: Harry Wentland Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Uma Shankar Cc: Ville Syrjälä Cc: Joshua Ashton Cc: Jani Nikula Cc: dri-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Acked-by: Pekka Paalanen Reviewed-By: Joshua Ashton Link: https://patchwork.freedesktop.org/patch/msgid/20230113162428.33874-2-harry.wentl...@amd.com Signed-off-by: Alex Deucher Cc: sta...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/gpu/drm/display/drm_hdmi_helper.c |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) --- a/drivers/gpu/drm/display/drm_hdmi_helper.c +++ b/drivers/gpu/drm/display/drm_hdmi_helper.c @@ -44,10 +44,8 @@ int drm_hdmi_infoframe_set_hdr_metadata( /* Sink EOTF is Bit map while infoframe is absolute values */ if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf, - connector->hdr_sink_metadata.hdmi_type1.eotf)) { - DRM_DEBUG_KMS("EOTF Not Supported\n"); - return -EINVAL; - } + connector->hdr_sink_metadata.hdmi_type1.eotf)) + DRM_DEBUG_KMS("Unknown EOTF %d\n", hdr_metadata->hdmi_metadata_type1.eotf); err = hdmi_drm_infoframe_init(frame); if (err < 0)
Re: [RFC PATCH 2/5] xen/grants: update initialization order of xen grant table
On Sun, Mar 12, 2023 at 08:01:54PM +0800, Huang Rui wrote: > The xen grant table will be initialied before parsing the PCI resources, > so xen_alloc_unpopulated_pages() ends up using a range from the PCI > window because Linux hasn't parsed the PCI information yet. > > So modify the initialization order to make sure the real PCI resources > are parsed before. Has this been tested on a domU to make sure the late grant table init doesn't interfere with PV devices getting setup? > Signed-off-by: Huang Rui > --- > arch/x86/xen/grant-table.c | 2 +- > drivers/xen/grant-table.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/xen/grant-table.c b/arch/x86/xen/grant-table.c > index 1e681bf62561..64a04d1e70f5 100644 > --- a/arch/x86/xen/grant-table.c > +++ b/arch/x86/xen/grant-table.c > @@ -165,5 +165,5 @@ static int __init xen_pvh_gnttab_setup(void) > } > /* Call it _before_ __gnttab_init as we need to initialize the > * xen_auto_xlat_grant_frames first. */ > -core_initcall(xen_pvh_gnttab_setup); > +fs_initcall_sync(); > #endif > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c > index e1ec725c2819..6382112f3473 100644 > --- a/drivers/xen/grant-table.c > +++ b/drivers/xen/grant-table.c > @@ -1680,4 +1680,4 @@ static int __gnttab_init(void) > } > /* Starts after core_initcall so that xen_pvh_gnttab_setup can be called > * beforehand to initialize xen_auto_xlat_grant_frames. */ Comment need to be updated, but I was thinking whether it won't be best to simply call xen_pvh_gnttab_setup() from __gnttab_init() itself when running as a PVH guest? Thanks, Roger.
[PATCH 6.2 045/141] drm/nouveau/kms/nv50: fix nv50_wndw_new_ prototype
From: Jiri Slaby (SUSE) [ Upstream commit 3638a820c5c3b52f327cebb174fd4274bee08aa7 ] gcc-13 warns about mismatching types for enums. That revealed switched arguments of nv50_wndw_new_(): drivers/gpu/drm/nouveau/dispnv50/wndw.c:696:1: error: conflicting types for 'nv50_wndw_new_' due to enum/integer mismatch; have 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, u32, enum nv50_disp_interlock_type, u32, struct nv50_wndw **)' drivers/gpu/drm/nouveau/dispnv50/wndw.h:36:5: note: previous declaration of 'nv50_wndw_new_' with type 'int(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *, int, const u32 *, enum nv50_disp_interlock_type, u32, u32, struct nv50_wndw **)' It can be barely visible, but the declaration says about the parameters in the middle: enum nv50_disp_interlock_type, u32 interlock_data, u32 heads, While the definition states differently: u32 heads, enum nv50_disp_interlock_type interlock_type, u32 interlock_data, Unify/fix the declaration to match the definition. Fixes: 53e0a3e70de6 ("drm/nouveau/kms/nv50-: simplify tracking of channel interlocks") Cc: Martin Liska Cc: Ben Skeggs Cc: Karol Herbst Cc: Lyude Paul Cc: David Airlie Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Jiri Slaby (SUSE) Signed-off-by: Karol Herbst Link: https://patchwork.freedesktop.org/patch/msgid/20221031114229.10289-1-jirisl...@kernel.org Signed-off-by: Sasha Levin --- drivers/gpu/drm/nouveau/dispnv50/wndw.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.h b/drivers/gpu/drm/nouveau/dispnv50/wndw.h index 591c852f326b9..76a6ae5d56526 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.h +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.h @@ -35,8 +35,9 @@ struct nv50_wndw { int nv50_wndw_new_(const struct nv50_wndw_func *, struct drm_device *, enum drm_plane_type, const char *name, int index, - const u32 *format, enum nv50_disp_interlock_type, - u32 interlock_data, u32 heads, struct nv50_wndw **); + const u32 *format, u32 heads, + enum nv50_disp_interlock_type, u32 interlock_data, + struct nv50_wndw **); void nv50_wndw_flush_set(struct nv50_wndw *, u32 *interlock, struct nv50_wndw_atom *); void nv50_wndw_flush_clr(struct nv50_wndw *, u32 *interlock, bool flush, -- 2.39.2
Re: [RFC PATCH 3/5] drm/amdgpu: set passthrough mode for xen pvh/hvm
On Sun, Mar 12, 2023 at 08:01:55PM +0800, Huang Rui wrote: > There is an second stage translation between the guest machine address > and host machine address in Xen PVH/HVM. The PCI bar address in the xen > guest kernel are not translated at the second stage on Xen PVH/HVM, so I'm confused by the sentence above, do you think it could be reworded or expanded to clarify? PCI BAR addresses are not in the guest kernel, but rather in the physical memory layout made available to the guest. Also, I'm unsure why xen_initial_domain() needs to be used in the conditional below: all PV domains handle addresses the same, so if it's not needed for a PV dom0 it's likely not needed for a PV domU either. Albeit it would help to know more about what AMDGPU_PASSTHROUGH_MODE implies. Thanks, Roger.
Re: [PATCH 15/21] dt-bindings: soc: mediatek: add display mutex for MT8365 SoC
On 15/03/2023 09:01, Alexandre Mergnat wrote: > Le ven. 10 mars 2023 à 09:39, Krzysztof Kozlowski > a écrit : >> >> On 09/03/2023 15:23, Alexandre Mergnat wrote: >>> Add compatible for the MT8365 SoC. >>> >>> Signed-off-by: Alexandre Mergnat >>> --- >>> Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml >>> b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml >>> index ca0ca549257d..931d66893dff 100644 >>> --- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml >>> +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml >>> @@ -34,6 +34,7 @@ properties: >>>- mediatek,mt8186-mdp3-mutex >>>- mediatek,mt8192-disp-mutex >>>- mediatek,mt8195-disp-mutex >>> + - mediatek,mt8365-disp-mutex >> >> All these look compatible. Either make mt8635 compatible with something >> or even rework entire list and make everything compatible. > > All of them use "mtk_mutec.c" but have specific data. > > .data = &mt8173_mutex_driver_data}, > { .compatible = "mediatek,mt8183-disp-mutex", > .data = &mt8183_mutex_driver_data}, > { .compatible = "mediatek,mt8186-disp-mutex", > .data = &mt8186_mutex_driver_data}, > { .compatible = "mediatek,mt8186-mdp3-mutex", > .data = &mt8186_mdp_mutex_driver_data}, > { .compatible = "mediatek,mt8192-disp-mutex", > .data = &mt8192_mutex_driver_data}, > { .compatible = "mediatek,mt8195-disp-mutex", > .data = &mt8195_mutex_driver_data}, > { .compatible = "mediatek,mt8365-disp-mutex", > .data = &mt8365_mutex_driver_data}, > > I don't see any SoC compatible with mt8635, and I have no idea which > rework you have in mind. Can you please point me an example ? You are right, I looked at wrong of_device_id. Best regards, Krzysztof
Re: [RFC 01/10] drm: Track clients by tgid and not tid
Am 15.03.23 um 10:52 schrieb Tvrtko Ursulin: Hi, On 14/03/2023 15:33, Christian König wrote: Am 14.03.23 um 15:18 schrieb Tvrtko Ursulin: From: Tvrtko Ursulin Thread group id (aka pid from userspace point of view) is a more interesting thing to show as an owner of a DRM fd, so track and show that instead of the thread id. In the next patch we will make the owner updated post file descriptor handover, which will also be tgid based to avoid ping-pong when multiple threads access the fd. Signed-off-by: Tvrtko Ursulin Cc: Zack Rusin Cc: linux-graphics-maintai...@vmware.com Cc: Alex Deucher Cc: "Christian König" Reviewed-by: Zack Rusin Reviewed-by: Christian König Should we push the already reviewed cleanups like this one to drm-misc-next? That makes sense even without the rest of the functionality and reduce the amount of patches re-send. I don't have the commit rights so if you could do that I certainly would not mind, thanks! I've just pushed this patch here to drm-misc-next. As soon as Daniel or Dave gives his ok as well I'm going to also push the second one. Regards, Christian. Regards, Tvrtko --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +- drivers/gpu/drm/drm_debugfs.c | 4 ++-- drivers/gpu/drm/drm_file.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index d8e683688daa..863cb668e000 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -969,7 +969,7 @@ static int amdgpu_debugfs_gem_info_show(struct seq_file *m, void *unused) * Therefore, we need to protect this ->comm access using RCU. */ rcu_read_lock(); - task = pid_task(file->pid, PIDTYPE_PID); + task = pid_task(file->pid, PIDTYPE_TGID); seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), task ? task->comm : ""); rcu_read_unlock(); diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c index 4f643a490dc3..4855230ba2c6 100644 --- a/drivers/gpu/drm/drm_debugfs.c +++ b/drivers/gpu/drm/drm_debugfs.c @@ -80,7 +80,7 @@ static int drm_clients_info(struct seq_file *m, void *data) seq_printf(m, "%20s %5s %3s master a %5s %10s\n", "command", - "pid", + "tgid", "dev", "uid", "magic"); @@ -94,7 +94,7 @@ static int drm_clients_info(struct seq_file *m, void *data) bool is_current_master = drm_is_current_master(priv); rcu_read_lock(); /* locks pid_task()->comm */ - task = pid_task(priv->pid, PIDTYPE_PID); + task = pid_task(priv->pid, PIDTYPE_TGID); uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID; seq_printf(m, "%20s %5d %3d %c %c %5d %10u\n", task ? task->comm : "", diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index a51ff8cee049..c1018c470047 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -156,7 +156,7 @@ struct drm_file *drm_file_alloc(struct drm_minor *minor) if (!file) return ERR_PTR(-ENOMEM); - file->pid = get_pid(task_pid(current)); + file->pid = get_pid(task_tgid(current)); file->minor = minor; /* for compatibility root is always authenticated */ diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c index d6baf73a6458..c0da89e16e6f 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c @@ -241,7 +241,7 @@ static int vmw_debugfs_gem_info_show(struct seq_file *m, void *unused) * Therefore, we need to protect this ->comm access using RCU. */ rcu_read_lock(); - task = pid_task(file->pid, PIDTYPE_PID); + task = pid_task(file->pid, PIDTYPE_TGID); seq_printf(m, "pid %8d command %s:\n", pid_nr(file->pid), task ? task->comm : ""); rcu_read_unlock();
Re: [PATCH 15/21] dt-bindings: soc: mediatek: add display mutex for MT8365 SoC
On 09/03/2023 15:23, Alexandre Mergnat wrote: > Add compatible for the MT8365 SoC. > > Signed-off-by: Alexandre Mergnat > --- Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v13 01/10] drm/shmem-helper: Switch to reservation lock
On 3/14/23 05:26, Dmitry Osipenko wrote: > @@ -633,7 +605,10 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object > *shmem, struct vm_area_struct > return ret; > } > > + dma_resv_lock(shmem->base.resv, NULL); > ret = drm_gem_shmem_get_pages(shmem); > + dma_resv_unlock(shmem->base.resv); Intel CI reported locking problem [1] here. It actually was also reported for v12, but I missed that report because of the other noisy reports. [1] https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_114671v2/shard-snb5/igt@prime_vgem@s...@rcs0.html The test does the following: 1. creates vgem 2. export it do dmabuf 3. mmaps dmabuf There is an obvious deadlock there. The DRM code assumes that mmap() is unlocked, while for dma-buf it's locked. I'll see how to fix it for v14. -- Best regards, Dmitry
Re: [PATCH v10 01/15] dma-buf/dma-fence: Add deadline awareness
On Fri, Mar 10, 2023 at 09:38:18AM -0800, Rob Clark wrote: > On Fri, Mar 10, 2023 at 7:45 AM Jonas Ådahl wrote: > > > > On Wed, Mar 08, 2023 at 07:52:52AM -0800, Rob Clark wrote: > > > From: Rob Clark > > > > > > Add a way to hint to the fence signaler of an upcoming deadline, such as > > > vblank, which the fence waiter would prefer not to miss. This is to aid > > > the fence signaler in making power management decisions, like boosting > > > frequency as the deadline approaches and awareness of missing deadlines > > > so that can be factored in to the frequency scaling. > > > > > > v2: Drop dma_fence::deadline and related logic to filter duplicate > > > deadlines, to avoid increasing dma_fence size. The fence-context > > > implementation will need similar logic to track deadlines of all > > > the fences on the same timeline. [ckoenig] > > > v3: Clarify locking wrt. set_deadline callback > > > v4: Clarify in docs comment that this is a hint > > > v5: Drop DMA_FENCE_FLAG_HAS_DEADLINE_BIT. > > > v6: More docs > > > v7: Fix typo, clarify past deadlines > > > > > > Signed-off-by: Rob Clark > > > Reviewed-by: Christian König > > > Acked-by: Pekka Paalanen > > > Reviewed-by: Bagas Sanjaya > > > --- > > > > Hi Rob! > > > > > Documentation/driver-api/dma-buf.rst | 6 +++ > > > drivers/dma-buf/dma-fence.c | 59 > > > include/linux/dma-fence.h| 22 +++ > > > 3 files changed, 87 insertions(+) > > > > > > diff --git a/Documentation/driver-api/dma-buf.rst > > > b/Documentation/driver-api/dma-buf.rst > > > index 622b8156d212..183e480d8cea 100644 > > > --- a/Documentation/driver-api/dma-buf.rst > > > +++ b/Documentation/driver-api/dma-buf.rst > > > @@ -164,6 +164,12 @@ DMA Fence Signalling Annotations > > > .. kernel-doc:: drivers/dma-buf/dma-fence.c > > > :doc: fence signalling annotation > > > > > > +DMA Fence Deadline Hints > > > + > > > + > > > +.. kernel-doc:: drivers/dma-buf/dma-fence.c > > > + :doc: deadline hints > > > + > > > DMA Fences Functions Reference > > > ~~ > > > > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > > > index 0de0482cd36e..f177c56269bb 100644 > > > --- a/drivers/dma-buf/dma-fence.c > > > +++ b/drivers/dma-buf/dma-fence.c > > > @@ -912,6 +912,65 @@ dma_fence_wait_any_timeout(struct dma_fence > > > **fences, uint32_t count, > > > } > > > EXPORT_SYMBOL(dma_fence_wait_any_timeout); > > > > > > +/** > > > + * DOC: deadline hints > > > + * > > > + * In an ideal world, it would be possible to pipeline a workload > > > sufficiently > > > + * that a utilization based device frequency governor could arrive at a > > > minimum > > > + * frequency that meets the requirements of the use-case, in order to > > > minimize > > > + * power consumption. But in the real world there are many workloads > > > which > > > + * defy this ideal. For example, but not limited to: > > > + * > > > + * * Workloads that ping-pong between device and CPU, with alternating > > > periods > > > + * of CPU waiting for device, and device waiting on CPU. This can > > > result in > > > + * devfreq and cpufreq seeing idle time in their respective domains > > > and in > > > + * result reduce frequency. > > > + * > > > + * * Workloads that interact with a periodic time based deadline, such > > > as double > > > + * buffered GPU rendering vs vblank sync'd page flipping. In this > > > scenario, > > > + * missing a vblank deadline results in an *increase* in idle time on > > > the GPU > > > + * (since it has to wait an additional vblank period), sending a > > > signal to > > > + * the GPU's devfreq to reduce frequency, when in fact the opposite is > > > what is > > > + * needed. > > > > This is the use case I'd like to get some better understanding about how > > this series intends to work, as the problematic scheduling behavior > > triggered by missed deadlines has plagued compositing display servers > > for a long time. > > > > I apologize, I'm not a GPU driver developer, nor an OpenGL driver > > developer, so I will need some hand holding when it comes to > > understanding exactly what piece of software is responsible for > > communicating what piece of information. > > > > > + * > > > + * To this end, deadline hint(s) can be set on a &dma_fence via > > > &dma_fence_set_deadline. > > > + * The deadline hint provides a way for the waiting driver, or > > > userspace, to > > > + * convey an appropriate sense of urgency to the signaling driver. > > > + * > > > + * A deadline hint is given in absolute ktime (CLOCK_MONOTONIC for > > > userspace > > > + * facing APIs). The time could either be some point in the future > > > (such as > > > + * the vblank based deadline for page-flipping, or the start of a > > > compositor's > > > + * composition cycle), or the current time to indicate an immediate > > > deadline > > > + * hint (Ie. fo
Re: [RFC PATCH 4/5] x86/xen: acpi registers gsi for xen pvh
On Sun, Mar 12, 2023 at 08:01:56PM +0800, Huang Rui wrote: > From: Chen Jiqian > > Add acpi_register_gsi_xen_pvh() to register gsi for PVH mode. > In addition to call acpi_register_gsi_ioapic(), it also setup > a map between gsi and vector in hypervisor side. So that, > when dgpu create an interrupt, hypervisor can correctly find > which guest domain to process interrupt by vector. The term 'dgpu' needs clarifying or replacing by a more generic naming. Also, I would like to be able to get away from requiring dom0 to register the GSIs in this way. If you take a look at Xen, there's code in the emulated IO-APIC available to dom0 that already does this registering (see vioapic_hwdom_map_gsi() in Xen). I think the problem here is that the GSI used by the device you want to passthrough has never had it's pin unmasked in the IO-APIC, and hence hasn't been registered. It would be helpful if you could state in the commit message what issue you are trying to solve by doing this registering here, I assume it is done in order to map the IRQ to a PIRQ, so later calls by the toolstack to bind it succeed. Would it be possible instead to perform the call to PHYSDEVOP_map_pirq in the toolstack itself if the PIRQ cannot be found? Thanks, Roger.
Re: [PATCH 19/21] drm/mediatek: dpi: add support for dpi clock
Hi, Alexandre: Alexandre Mergnat 於 2023年3月15日 週三 下午5:36寫道: > > Le lun. 13 mars 2023 à 16:21, Chun-Kuang Hu a écrit > : > > > > > > > > + dpi->dpi_clk = devm_clk_get_optional(dev, "dpi"); > > > > For MT8365, DPI clock is not optional, so make sure that MT8365 DPI > > should have this clock. > > This should be check and notified at build time thanks to the > device-tree binding. Do you prefer this?: > #IF MT8365_SOC > dpi->dpi_clk = devm_clk_get(dev, "dpi"); Add one member in struct mtk_dpi_conf for dpi_clk. In mtk_dpi_of_ids[], add one item for mt8365 { .compatible = "mediatek,mt8365-dpi", .data = &mt8365_conf, }, Regards, Chun-Kuang. > > Regards, > Alex
Re: [RFC PATCH 5/5] xen/privcmd: add IOCTL_PRIVCMD_GSI_FROM_IRQ
On Sun, Mar 12, 2023 at 08:01:57PM +0800, Huang Rui wrote: > From: Chen Jiqian > > When hypervisor get an interrupt, it needs interrupt's > gsi number instead of irq number. Gsi number is unique > in xen, but irq number is only unique in one domain. > So, we need to record the relationship between irq and > gsi when dom0 initialized pci devices, and provide syscall > IOCTL_PRIVCMD_GSI_FROM_IRQ to translate irq to gsi. So > that, we can map pirq successfully in hypervisor side. GSI is not only unique in Xen, it's an ACPI provided value that's unique in the platform. The text above make it look like GSI is some kind of internal Xen reference to an interrupt, but it's not. How does a PV domain deal with this? I would assume there Linux will also end up with IRQ != GSI, and hence will need some kind of translation? Thanks, Roger.
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
Am 15.03.23 um 16:09 schrieb Steven Rostedt: On Wed, 8 Mar 2023 07:17:38 +0100 Christian König wrote: Am 08.03.23 um 03:26 schrieb Steven Rostedt: On Tue, 7 Mar 2023 21:22:23 -0500 Steven Rostedt wrote: Looks like there was a lock possibly used after free. But as commit 9bff18d13473a9fdf81d5158248472a9d8ecf2bd ("drm/ttm: use per BO cleanup workers") changed a lot of this code, I figured it may be the culprit. If I bothered to look at the second warning after this one (I usually stop after the first), it appears to state there was a use after free issue. Yeah, that looks like the reference count was somehow messed up. What test case/environment do you run to trigger this? Thanks for the notice, I'm still getting this on Linus's latest tree. This must be some reference counting issue which only happens in your particular use case. We have tested this quite extensively and couldn't reproduce it so far. Can you apply this code snippet here and see if you get any warning in the system logs? diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 459f1b4440da..efc390bfd69c 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -314,6 +314,7 @@ static void ttm_bo_delayed_delete(struct work_struct *work) dma_resv_lock(bo->base.resv, NULL); ttm_bo_cleanup_memtype_use(bo); dma_resv_unlock(bo->base.resv); + bo->delayed_delete.func = NULL; ttm_bo_put(bo); } @@ -327,6 +328,8 @@ static void ttm_bo_release(struct kref *kref) WARN_ON_ONCE(bo->pin_count); WARN_ON_ONCE(bo->bulk_move); + WARN_ON(bo->delayed_delete.func != NULL); + if (!bo->deleted) { ret = ttm_bo_individualize_resv(bo); if (ret) { Thanks, Christian. [ 230.530222] [ cut here ] [ 230.569795] DEBUG_LOCKS_WARN_ON(lock->magic != lock) [ 230.569957] WARNING: CPU: 0 PID: 212 at kernel/locking/mutex.c:582 __ww_mutex_lock.constprop.0+0x62a/0x1300 [ 230.612599] Modules linked in: [ 230.632144] CPU: 0 PID: 212 Comm: kworker/0:8H Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #992 [ 230.654939] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 230.678866] Workqueue: ttm ttm_bo_delayed_delete [ 230.699452] EIP: __ww_mutex_lock.constprop.0+0x62a/0x1300 [ 230.720582] Code: e8 3b 9a 95 ff 85 c0 0f 84 61 fa ff ff 8b 0d 58 bc 3a c4 85 c9 0f 85 53 fa ff ff 68 54 98 06 c4 68 b7 b6 04 c4 e8 46 af 40 ff <0f> 0b 58 5a e9 3b fa ff ff 8d 74 26 00 90 a1 ec 47 b0 c4 85 c0 75 [ 230.768336] EAX: 0028 EBX: ECX: c51abdd8 EDX: 0002 [ 230.792001] ESI: EDI: c53856bc EBP: c51abf00 ESP: c51abeac [ 230.815944] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010246 [ 230.840033] CR0: 80050033 CR2: ff9ff000 CR3: 04506000 CR4: 00150ef0 [ 230.864059] Call Trace: [ 230.886369] ? ttm_bo_delayed_delete+0x30/0x94 [ 230.909902] ww_mutex_lock+0x32/0x94 [ 230.932550] ttm_bo_delayed_delete+0x30/0x94 [ 230.955798] process_one_work+0x21a/0x484 [ 230.979335] worker_thread+0x14a/0x39c [ 231.002258] kthread+0xea/0x10c [ 231.024769] ? process_one_work+0x484/0x484 [ 231.047870] ? kthread_complete_and_exit+0x1c/0x1c [ 231.071498] ret_from_fork+0x1c/0x28 [ 231.094701] irq event stamp: 4023 [ 231.117272] hardirqs last enabled at (4023): [] _raw_spin_unlock_irqrestore+0x2d/0x58 [ 231.143217] hardirqs last disabled at (4022): [] kvfree_call_rcu+0x155/0x2ec [ 231.166058] softirqs last enabled at (3460): [] __do_softirq+0x2c3/0x3bb [ 231.183104] softirqs last disabled at (3455): [] call_on_stack+0x45/0x4c [ 231.200336] ---[ end trace ]--- [ 231.216572] [ cut here ] This is preventing me from adding any of my own patches on v6.3-rcX due to this bug failing my tests. Which means I can't add anything to linux-next until this is fixed! -- Steve
Re: [PATCH] drm/vmwgfx: Fix src/dst_pitch confusion
From: Martin Krastev We reviewers botched that one. Reviewed-by: Martin Krastev Regards, Martin On 14.03.23 г. 23:14 ч., Zack Rusin wrote: From: Zack Rusin The src/dst_pitch got mixed up during the rework of the function, make sure the offset's refer to the correct one. Spotted by clang: Clang warns (or errors with CONFIG_WERROR): drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:509:29: error: variable 'dst_pitch' is uninitialized when used here [-Werror,-Wuninitialized] src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; ^ drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c:492:26: note: initialize the variable 'dst_pitch' to silence this warning s32 src_pitch, dst_pitch; ^ = 0 1 error generated. Signed-off-by: Zack Rusin Link: https://github.com/ClangBuiltLinux/linux/issues/1811 Reported-by: Nathan Chancellor Reported-by: Dave Airlie Fixes: 39985eea5a6d ("drm/vmwgfx: Abstract placement selection") --- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c index d79a6eccfaa4..ba0c0e12cfe9 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c @@ -506,11 +506,11 @@ static void vmw_stdu_bo_cpu_commit(struct vmw_kms_dirty *dirty) /* Assume we are blitting from Guest (bo) to Host (display_srf) */ src_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp; src_bo = &stdu->display_srf->res.guest_memory_bo->tbo; - src_offset = ddirty->top * dst_pitch + ddirty->left * stdu->cpp; + src_offset = ddirty->top * src_pitch + ddirty->left * stdu->cpp; dst_pitch = ddirty->pitch; dst_bo = &ddirty->buf->tbo; - dst_offset = ddirty->fb_top * src_pitch + ddirty->fb_left * stdu->cpp; + dst_offset = ddirty->fb_top * dst_pitch + ddirty->fb_left * stdu->cpp; (void) vmw_bo_cpu_blit(dst_bo, dst_offset, dst_pitch, src_bo, src_offset, src_pitch,
Re: [Intel-gfx] [PATCH v2] drm/i915/mtl: Disable stolen memory backed FB for A0
On 10.03.2023 10:45, Nirmoy Das wrote: Stolen memory is not usable for MTL A0 stepping beyond certain access size and we have no control over userspace access size of /dev/fb which can be backed by stolen memory. So disable stolen memory backed fb by setting i915->dsm.usable_size to zero. v2: remove hsdes reference and fix commit message(Andi) Cc: Matthew Auld Cc: Andi Shyti Cc: Daniele Ceraolo Spurio Cc: Lucas De Marchi Signed-off-by: Nirmoy Das Reviewed-by: Andi Shyti Reviewed-by: Andrzej Hajda Regards Andrzej --- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index d8e06e783e30..bf0f0a5f2a5c 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -535,6 +535,15 @@ static int i915_gem_init_stolen(struct intel_memory_region *mem) /* Basic memrange allocator for stolen space. */ drm_mm_init(&i915->mm.stolen, 0, i915->dsm.usable_size); + /* +* Access to stolen lmem beyond certain size for MTL A0 stepping +* would crash the machine. Disable stolen lmem for userspace access +* by setting usable_size to zero. +*/ + if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) || + IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) + i915->dsm.usable_size = 0; + return 0; }
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
On Wed, 15 Mar 2023 16:25:11 +0100 Christian König wrote: > >> > >> Thanks for the notice, > > I'm still getting this on Linus's latest tree. > > This must be some reference counting issue which only happens in your > particular use case. We have tested this quite extensively and couldn't > reproduce it so far. Have you tried 32 bit with my config. I also sent a link to your previous email that gives access to the VM image I'm using that is triggering this issue. Here it is again: The libvirt xml file is here: https://rostedt.org/vm-images/tracetest-32.xml and the VM image itself is here: https://rostedt.org/vm-images/tracetest-32.qcow2.bz2 > > Can you apply this code snippet here and see if you get any warning in > the system logs? > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 459f1b4440da..efc390bfd69c 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -314,6 +314,7 @@ static void ttm_bo_delayed_delete(struct work_struct > *work) > dma_resv_lock(bo->base.resv, NULL); > ttm_bo_cleanup_memtype_use(bo); > dma_resv_unlock(bo->base.resv); > + bo->delayed_delete.func = NULL; > ttm_bo_put(bo); > } > > @@ -327,6 +328,8 @@ static void ttm_bo_release(struct kref *kref) > WARN_ON_ONCE(bo->pin_count); > WARN_ON_ONCE(bo->bulk_move); > > + WARN_ON(bo->delayed_delete.func != NULL); > + > if (!bo->deleted) { > ret = ttm_bo_individualize_resv(bo); > if (ret) { > The WARN_ON triggered: [ 21.481449] mpls_gso: MPLS GSO support [ 21.488795] IPI shorthand broadcast: enabled [ 21.488873] [ cut here ] [ 21.490101] [ cut here ] [ 21.491693] WARNING: CPU: 1 PID: 38 at drivers/gpu/drm/ttm/ttm_bo.c:332 ttm_bo_release+0x2ac/0x2fc <<< Line of the added WARN_ON() [ 21.492940] refcount_t: underflow; use-after-free. [ 21.492965] WARNING: CPU: 0 PID: 84 at lib/refcount.c:28 refcount_warn_saturate+0xb6/0xfc [ 21.496116] Modules linked in: [ 21.497197] Modules linked in: [ 21.500105] CPU: 1 PID: 38 Comm: kworker/1:1 Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 [ 21.500789] CPU: 0 PID: 84 Comm: kworker/0:1H Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 [ 21.501882] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 21.503533] sched_clock: Marking stable (20788024762, 714243692)->(22140778105, -638509651) [ 21.504080] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 21.504089] Workqueue: ttm ttm_bo_delayed_delete [ 21.507196] Workqueue: events drm_fb_helper_damage_work [ 21.509235] [ 21.510291] registered taskstats version 1 [ 21.510302] Running ring buffer tests... [ 21.511792] [ 21.513870] EIP: refcount_warn_saturate+0xb6/0xfc [ 21.515261] EIP: ttm_bo_release+0x2ac/0x2fc [ 21.516566] Code: 68 00 27 0c d8 e8 36 3b aa ff 0f 0b 58 c9 c3 90 80 3d 41 c2 37 d8 00 75 8a c6 05 41 c2 37 d8 01 68 2c 27 0c d8 e8 16 3b aa ff <0f> 0b 59 c9 c3 80 3d 3f c2 37 d8 00 0f 85 67 ff ff ff c6 05 3f c2 [ 21.516998] Code: ff 8d b4 26 00 00 00 00 66 90 0f 0b 8b 43 10 85 c0 0f 84 a1 fd ff ff 8d 76 00 0f 0b 8b 43 28 85 c0 0f 84 9c fd ff ff 8d 76 00 <0f> 0b e9 92 fd ff ff 8d b4 26 00 00 00 00 66 90 c7 43 18 00 00 00 [ 21.517905] EAX: 0026 EBX: c129d150 ECX: 0040 EDX: 0002 [ 21.518987] EAX: d78c8550 EBX: c129d134 ECX: c129d134 EDX: 0001 [ 21.519337] ESI: c129d0bc EDI: f6f91200 EBP: c2b8bf18 ESP: c2b8bf14 [ 21.520617] ESI: c129d000 EDI: c126a7a0 EBP: c1839c24 ESP: c1839bec [ 21.521546] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 [ 21.526154] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 [ 21.526162] CR0: 80050033 CR2: CR3: 18506000 CR4: 00150ef0 [ 21.526166] Call Trace: [ 21.526189] ? ww_mutex_unlock+0x3a/0x94 [ 21.530300] CR0: 80050033 CR2: ff9ff000 CR3: 18506000 CR4: 00150ef0 [ 21.531722] ? ttm_bo_cleanup_refs+0xc4/0x1e0 [ 21.533114] Call Trace: [ 21.534516] ttm_mem_evict_first+0x3d3/0x568 [ 21.535901] ttm_bo_delayed_delete+0x9c/0xa4 [ 21.537391] ? kfree+0x6b/0xdc [ 21.538901] process_one_work+0x21a/0x484 [ 21.540279] ? ttm_range_man_alloc+0xe0/0xec [ 21.540854] worker_thread+0x14a/0x39c [ 21.541714] ? ttm_range_man_fini_nocheck+0xe8/0xe8 [ 21.543332] kthread+0xea/0x10c [ 21.544301] ttm_bo_mem_space+0x1d0/0x1e4 [ 21.544942] ? process_one_work+0x484/0x484 [ 21.545887] ttm_bo_validate+0xc5/0x19c [ 21.546986] ? kthread_complete_and_exit+0x1c/0x1c [ 21.547680] ttm_bo_init_reserved+0x15e/0x1fc [ 21.548716] ret_from_fork+0x1c/0x28 [ 21.549650] qxl_bo_create+0x145/0x20c Note, this is all on boot up before user space is running. -- Steve
Re: [PATCH 21/21] panel: startek-kd070fhfid015: add support of this display
Hi Neil, Le jeu. 9 mars 2023 à 15:51, Neil Armstrong a écrit : > > + > > +#include > > Is this include needed ? Seems not, I remove it. > > + struct gpio_desc *enable_gpio; > > + struct gpio_desc *reset_gpio; > > + struct gpio_desc *dcdc_en_gpio; > > Isn't this "DCDC" a regulator ??? According to the panel datasheet, this is an input signal needed passing through TFT connector (PIN30): "Power IC supply enable, High active" > > + /* Interface setting, video mode */ > > + ret = mipi_dsi_generic_write(dsi, > > + (u8[]){DSI_REG_IS, 0x14, 0x08, 0x00, 0x22, 0x00}, 6); > > + if (ret < 0) { > > + dev_err(dev, "failed to set display interface setting: %d\n", > > + ret); > > + return ret; > > + } > > Use mipi_dsi_generic_write_seq(dsi, DSI_REG_IS, 0x14, 0x08, 0x00, 0x22, > 0x00); instead, > it's simpler and doesn't need explicit return check. "mipi_dsi_generic_write_seq" doesn't exist but I will add it below "mipi_dsi_dcs_write_seq" in drm_mipi_dsi.h I must keep the return check to jump out of "stk_panel_init" if something goes wrong, but I can drop the error print. > > +static int stk_panel_enable(struct drm_panel *panel) > > +{ > > + struct stk_panel *stk = to_stk_panel(panel); > > + > > + if (stk->enabled) > > + return 0; > > + > > + backlight_enable(stk->backlight); > > + > > + stk->enabled = true; > > + > > + return 0; > > +} > > Is this really needed ? the backlight core will enable it automatically. Ok, it's managed by drm_panel.c driver. Then I can also remove stuff from stk_panel_disable. Regards, Alex
[PATCH 0/6] drm/fbdev-generic: Mandatory shadow buffering
After adding fbdev-dma and converting drivers, all users of fbdev-generic require shadow buffering. Make it mandatory and remove all other codepaths. This change greatly simplifies the code for generic fbdev emulation. It will work with any driver that supports GEM's vmap and vunmap. The change further allows for a number of cleanups and fixes. The flag prefer_shadow_fbdev is unused and to be removed. Probing in fbdev-generic is now simple enough to roll back if it fails. Further simplify the code for exporting the framebuffer's physical address. Finally rename the symbols to follow other fbdev emulation. Thomas Zimmermann (6): drm/fbdev-generic: Always use shadow buffering drm/fbdev-generic: Remove unused prefer_shadow_fbdev flag drm/fb-helper: Export drm_fb_helper_release_info() drm/fbdev-generic: Clean up after failed probing drm/fb-helper: Consolidate CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM drm/fbdev-generic: Rename symbols drivers/gpu/drm/drm_fb_helper.c | 55 +++--- drivers/gpu/drm/drm_fbdev_dma.c | 9 +- drivers/gpu/drm/drm_fbdev_generic.c | 280 +--- drivers/gpu/drm/tiny/bochs.c| 1 - drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 1 - include/drm/drm_fb_helper.h | 14 +- include/drm/drm_mode_config.h | 7 - 7 files changed, 125 insertions(+), 242 deletions(-) base-commit: ec0708e846b819c8d5b642de42448a87d7526564 -- 2.39.2
[PATCH 2/6] drm/fbdev-generic: Remove unused prefer_shadow_fbdev flag
Remove the flag prefer_shadow_fbdev from struct drm_mode_config. Drivers set this flag to enable shadow buffering in the generic fbdev emulation. Such shadow buffering is now mandatory, so the flag is unused. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/tiny/bochs.c| 1 - drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 1 - include/drm/drm_mode_config.h | 7 --- 3 files changed, 9 deletions(-) diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c index 024346054c70..d254679a136e 100644 --- a/drivers/gpu/drm/tiny/bochs.c +++ b/drivers/gpu/drm/tiny/bochs.c @@ -545,7 +545,6 @@ static int bochs_kms_init(struct bochs_device *bochs) bochs->dev->mode_config.preferred_depth = 24; bochs->dev->mode_config.prefer_shadow = 0; - bochs->dev->mode_config.prefer_shadow_fbdev = 1; bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true; bochs->dev->mode_config.funcs = &bochs_mode_funcs; diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 84d6380b9895..5162a7a12792 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c @@ -2112,7 +2112,6 @@ int vmw_kms_init(struct vmw_private *dev_priv) dev->mode_config.max_width = dev_priv->texture_max_width; dev->mode_config.max_height = dev_priv->texture_max_height; dev->mode_config.preferred_depth = dev_priv->assume_16bpp ? 16 : 32; - dev->mode_config.prefer_shadow_fbdev = !dev_priv->has_mob; drm_mode_create_suggested_offset_properties(dev); vmw_kms_create_hotplug_mode_update_property(dev_priv); diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index e5b053001d22..973119a9176b 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -890,13 +890,6 @@ struct drm_mode_config { /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow; - /** -* @prefer_shadow_fbdev: -* -* Hint to framebuffer emulation to prefer shadow-fb rendering. -*/ - bool prefer_shadow_fbdev; - /** * @quirk_addfb_prefer_xbgr_30bpp: * -- 2.39.2
[PATCH 3/6] drm/fb-helper: Export drm_fb_helper_release_info()
Export the fb_info release code as drm_fb_helper_release_info(). Will help with cleaning up failed fbdev probing. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_fb_helper.c | 33 - include/drm/drm_fb_helper.h | 5 + 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index a39998047f8a..7e96ed9efdb5 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -538,6 +538,29 @@ struct fb_info *drm_fb_helper_alloc_info(struct drm_fb_helper *fb_helper) } EXPORT_SYMBOL(drm_fb_helper_alloc_info); +/** + * drm_fb_helper_release_info - release fb_info and its members + * @fb_helper: driver-allocated fbdev helper + * + * A helper to release fb_info and the member cmap. Drivers do not + * need to release the allocated fb_info structure themselves, this is + * automatically done when calling drm_fb_helper_fini(). + */ +void drm_fb_helper_release_info(struct drm_fb_helper *fb_helper) +{ + struct fb_info *info = fb_helper->info; + + if (!info) + return; + + fb_helper->info = NULL; + + if (info->cmap.len) + fb_dealloc_cmap(&info->cmap); + framebuffer_release(info); +} +EXPORT_SYMBOL(drm_fb_helper_release_info); + /** * drm_fb_helper_unregister_info - unregister fb_info framebuffer device * @fb_helper: driver-allocated fbdev helper, can be NULL @@ -561,8 +584,6 @@ EXPORT_SYMBOL(drm_fb_helper_unregister_info); */ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper) { - struct fb_info *info; - if (!fb_helper) return; @@ -574,13 +595,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper) cancel_work_sync(&fb_helper->resume_work); cancel_work_sync(&fb_helper->damage_work); - info = fb_helper->info; - if (info) { - if (info->cmap.len) - fb_dealloc_cmap(&info->cmap); - framebuffer_release(info); - } - fb_helper->info = NULL; + drm_fb_helper_release_info(fb_helper); mutex_lock(&kernel_fb_helper_lock); if (!list_empty(&fb_helper->kernel_fb_list)) { diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 013654de3fc5..c5822ec2fdd1 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -256,6 +256,7 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper); struct fb_info *drm_fb_helper_alloc_info(struct drm_fb_helper *fb_helper); +void drm_fb_helper_release_info(struct drm_fb_helper *fb_helper); void drm_fb_helper_unregister_info(struct drm_fb_helper *fb_helper); void drm_fb_helper_fill_info(struct fb_info *info, struct drm_fb_helper *fb_helper, @@ -365,6 +366,10 @@ drm_fb_helper_alloc_info(struct drm_fb_helper *fb_helper) return NULL; } +static inline void drm_fb_helper_release_info(struct drm_fb_helper *fb_helper) +{ +} + static inline void drm_fb_helper_unregister_info(struct drm_fb_helper *fb_helper) { } -- 2.39.2
[PATCH 5/6] drm/fb-helper: Consolidate CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM
Consolidate all handling of CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM by making the module parameter optional in drm_fb_helper.c. Without the config option, modules can set smem_start in struct fb_info for internal usage, but not export if to userspace. The address can only be exported by enabling the option and setting the module parameter. Also update the comment. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_fb_helper.c | 22 -- drivers/gpu/drm/drm_fbdev_dma.c | 9 + include/drm/drm_fb_helper.h | 9 - 3 files changed, 9 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 7e96ed9efdb5..b536b472f3ef 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -60,16 +60,17 @@ MODULE_PARM_DESC(drm_fbdev_overalloc, * In order to keep user-space compatibility, we want in certain use-cases * to keep leaking the fbdev physical address to the user-space program * handling the fbdev buffer. - * This is a bad habit essentially kept into closed source opengl driver - * that should really be moved into open-source upstream projects instead - * of using legacy physical addresses in user space to communicate with - * other out-of-tree kernel modules. + * + * This is a bad habit, essentially kept to support closed-source OpenGL + * drivers that should really be moved into open-source upstream projects + * instead of using legacy physical addresses in user space to communicate + * with other out-of-tree kernel modules. * * This module_param *should* be removed as soon as possible and be * considered as a broken and legacy behaviour from a modern fbdev device. */ -#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) static bool drm_leak_fbdev_smem; +#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) module_param_unsafe(drm_leak_fbdev_smem, bool, 0600); MODULE_PARM_DESC(drm_leak_fbdev_smem, "Allow unsafe leaking fbdev physical smem address [default=false]"); @@ -1979,10 +1980,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper) return ret; } -#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) - fb_helper->hint_leak_smem_start = drm_leak_fbdev_smem; -#endif - /* push down into drivers */ ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes); if (ret < 0) @@ -2181,11 +2178,8 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper) info = fb_helper->info; info->var.pixclock = 0; - /* Shamelessly allow physical address leaking to userspace */ -#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) - if (!fb_helper->hint_leak_smem_start) -#endif - /* don't leak any physical addresses to userspace */ + + if (!drm_leak_fbdev_smem) info->flags |= FBINFO_HIDE_SMEM_START; /* Need to drop locks to avoid recursive deadlock in diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c index cf553ac12a0f..728deffcc0d9 100644 --- a/drivers/gpu/drm/drm_fbdev_dma.c +++ b/drivers/gpu/drm/drm_fbdev_dma.c @@ -136,16 +136,9 @@ static int drm_fbdev_dma_helper_fb_probe(struct drm_fb_helper *fb_helper, info->flags |= FBINFO_READS_FAST; /* signal caching */ info->screen_size = sizes->surface_height * fb->pitches[0]; info->screen_buffer = map.vaddr; + info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer)); info->fix.smem_len = info->screen_size; -#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM) - /* -* Shamelessly leak the physical address to user-space. -*/ - if (fb_helper->hint_leak_smem_start && !info->fix.smem_start) - info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer)); -#endif - return 0; err_drm_client_buffer_vunmap: diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index c5822ec2fdd1..72032c354a30 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -195,15 +195,6 @@ struct drm_fb_helper { */ int preferred_bpp; - /** -* @hint_leak_smem_start: -* -* Hint to the fbdev emulation to store the framebuffer's physical -* address in struct &fb_info.fix.smem_start. If the hint is unset, -* the smem_start field should always be cleared to zero. -*/ - bool hint_leak_smem_start; - #ifdef CONFIG_FB_DEFERRED_IO /** * @fbdefio: -- 2.39.2
[PATCH 4/6] drm/fbdev-generic: Clean up after failed probing
Clean up fbdev and client state if the probe function fails. It used to leak allocated resources. Also reorder the individual steps to simplify cleanup. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_fbdev_generic.c | 46 - 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index e48a8e82378d..0d08ddbe9608 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -74,8 +75,9 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, struct drm_client_dev *client = &fb_helper->client; struct drm_device *dev = fb_helper->dev; struct drm_client_buffer *buffer; - struct drm_framebuffer *fb; struct fb_info *info; + size_t screen_size; + void *screen_buffer; u32 format; int ret; @@ -91,36 +93,52 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, fb_helper->buffer = buffer; fb_helper->fb = buffer->fb; - fb = buffer->fb; + + screen_size = buffer->gem->size; + screen_buffer = vzalloc(screen_size); + if (!screen_buffer) { + ret = -ENOMEM; + goto err_drm_client_framebuffer_delete; + } info = drm_fb_helper_alloc_info(fb_helper); - if (IS_ERR(info)) - return PTR_ERR(info); + if (IS_ERR(info)) { + ret = PTR_ERR(info); + goto err_vfree; + } + + drm_fb_helper_fill_info(info, fb_helper, sizes); info->fbops = &drm_fbdev_fb_ops; - info->screen_size = sizes->surface_height * fb->pitches[0]; - info->fix.smem_len = info->screen_size; info->flags = FBINFO_DEFAULT; - drm_fb_helper_fill_info(info, fb_helper, sizes); - - info->screen_buffer = vzalloc(info->screen_size); - if (!info->screen_buffer) - return -ENOMEM; + /* screen */ info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST; - + info->screen_size = screen_size; + info->screen_buffer = screen_buffer; info->fix.smem_start = page_to_phys(vmalloc_to_page(info->screen_buffer)); + info->fix.smem_len = info->screen_size; - /* Set a default deferred I/O handler */ + /* deferred I/O */ fb_helper->fbdefio.delay = HZ / 20; fb_helper->fbdefio.deferred_io = drm_fb_helper_deferred_io; info->fbdefio = &fb_helper->fbdefio; ret = fb_deferred_io_init(info); if (ret) - return ret; + goto err_drm_fb_helper_release_info; return 0; + +err_drm_fb_helper_release_info: + drm_fb_helper_release_info(fb_helper); +err_vfree: + vfree(screen_buffer); +err_drm_client_framebuffer_delete: + fb_helper->fb = NULL; + fb_helper->buffer = NULL; + drm_client_framebuffer_delete(buffer); + return ret; } static void drm_fbdev_damage_blit_real(struct drm_fb_helper *fb_helper, -- 2.39.2
[PATCH 6/6] drm/fbdev-generic: Rename symbols
Rename symbols to match the style of other fbdev-emulation source code. No functional changes. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_fbdev_generic.c | 66 ++--- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 0d08ddbe9608..e70e3138a78e 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -13,7 +13,7 @@ #include /* @user: 1=userspace, 0=fbcon */ -static int drm_fbdev_fb_open(struct fb_info *info, int user) +static int drm_fbdev_generic_fb_open(struct fb_info *info, int user) { struct drm_fb_helper *fb_helper = info->par; @@ -24,7 +24,7 @@ static int drm_fbdev_fb_open(struct fb_info *info, int user) return 0; } -static int drm_fbdev_fb_release(struct fb_info *info, int user) +static int drm_fbdev_generic_fb_release(struct fb_info *info, int user) { struct drm_fb_helper *fb_helper = info->par; @@ -34,7 +34,7 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user) return 0; } -static void drm_fbdev_fb_destroy(struct fb_info *info) +static void drm_fbdev_generic_fb_destroy(struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; void *shadow = info->screen_buffer; @@ -52,10 +52,10 @@ static void drm_fbdev_fb_destroy(struct fb_info *info) kfree(fb_helper); } -static const struct fb_ops drm_fbdev_fb_ops = { +static const struct fb_ops drm_fbdev_generic_fb_ops = { .owner = THIS_MODULE, - .fb_open= drm_fbdev_fb_open, - .fb_release = drm_fbdev_fb_release, + .fb_open= drm_fbdev_generic_fb_open, + .fb_release = drm_fbdev_generic_fb_release, .fb_read= drm_fb_helper_sys_read, .fb_write = drm_fb_helper_sys_write, DRM_FB_HELPER_DEFAULT_OPS, @@ -63,14 +63,14 @@ static const struct fb_ops drm_fbdev_fb_ops = { .fb_copyarea= drm_fb_helper_sys_copyarea, .fb_imageblit = drm_fb_helper_sys_imageblit, .fb_mmap= fb_deferred_io_mmap, - .fb_destroy = drm_fbdev_fb_destroy, + .fb_destroy = drm_fbdev_generic_fb_destroy, }; /* * This function uses the client API to create a framebuffer backed by a dumb buffer. */ -static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, - struct drm_fb_helper_surface_size *sizes) +static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper, +struct drm_fb_helper_surface_size *sizes) { struct drm_client_dev *client = &fb_helper->client; struct drm_device *dev = fb_helper->dev; @@ -109,7 +109,7 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, drm_fb_helper_fill_info(info, fb_helper, sizes); - info->fbops = &drm_fbdev_fb_ops; + info->fbops = &drm_fbdev_generic_fb_ops; info->flags = FBINFO_DEFAULT; /* screen */ @@ -141,9 +141,9 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, return ret; } -static void drm_fbdev_damage_blit_real(struct drm_fb_helper *fb_helper, - struct drm_clip_rect *clip, - struct iosys_map *dst) +static void drm_fbdev_generic_damage_blit_real(struct drm_fb_helper *fb_helper, + struct drm_clip_rect *clip, + struct iosys_map *dst) { struct drm_framebuffer *fb = fb_helper->fb; size_t offset = clip->y1 * fb->pitches[0]; @@ -180,8 +180,8 @@ static void drm_fbdev_damage_blit_real(struct drm_fb_helper *fb_helper, } } -static int drm_fbdev_damage_blit(struct drm_fb_helper *fb_helper, -struct drm_clip_rect *clip) +static int drm_fbdev_generic_damage_blit(struct drm_fb_helper *fb_helper, +struct drm_clip_rect *clip) { struct drm_client_buffer *buffer = fb_helper->buffer; struct iosys_map map, dst; @@ -205,7 +205,7 @@ static int drm_fbdev_damage_blit(struct drm_fb_helper *fb_helper, goto out; dst = map; - drm_fbdev_damage_blit_real(fb_helper, clip, &dst); + drm_fbdev_generic_damage_blit_real(fb_helper, clip, &dst); drm_client_buffer_vunmap(buffer); @@ -215,7 +215,8 @@ static int drm_fbdev_damage_blit(struct drm_fb_helper *fb_helper, return ret; } -static int drm_fbdev_fb_dirty(struct drm_fb_helper *helper, struct drm_clip_rect *clip) +static int drm_fbdev_generic_helper_fb_dirty(struct drm_fb_helper *helper, +struct drm_clip_rect *clip) { struct drm_device *dev = helper->dev; int ret; @@ -224,7 +225,7 @@ static int drm_fbdev_fb_dirty(struct drm_fb_helper *
[PATCH 1/6] drm/fbdev-generic: Always use shadow buffering
Remove all codepaths that implement fbdev output directly on GEM buffers. Always allocate a shadow buffer in system memory and set up deferred I/O for mmap. The fbdev code that operated directly on GEM buffers was used by drivers based on GEM DMA helpers. Those drivers have been migrated to use fbdev-dma, a dedicated fbdev emulation for DMA memory. All remaining users of fbdev-generic require shadow buffering. Memory management of the remaining callers uses TTM, GEM SHMEM helpers or a variant of GEM DMA helpers that is incompatible with fbdev-dma. Therefore remove the unused codepaths from fbdev-generic and simplify the code. Using a shadow buffer with deferred I/O is probably the best case for most remaining callers. Some of the TTM-based drivers might benefit from a dedicated fbdev emulation that operates directly on the driver's video memory. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_fbdev_generic.c | 184 +--- 1 file changed, 30 insertions(+), 154 deletions(-) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 4d6325e91565..e48a8e82378d 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -11,16 +11,6 @@ #include -static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper) -{ - struct drm_device *dev = fb_helper->dev; - struct drm_framebuffer *fb = fb_helper->fb; - - return dev->mode_config.prefer_shadow_fbdev || - dev->mode_config.prefer_shadow || - fb->funcs->dirty; -} - /* @user: 1=userspace, 0=fbcon */ static int drm_fbdev_fb_open(struct fb_info *info, int user) { @@ -46,115 +36,33 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user) static void drm_fbdev_fb_destroy(struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; - void *shadow = NULL; + void *shadow = info->screen_buffer; if (!fb_helper->dev) return; - if (info->fbdefio) - fb_deferred_io_cleanup(info); - if (drm_fbdev_use_shadow_fb(fb_helper)) - shadow = info->screen_buffer; - + fb_deferred_io_cleanup(info); drm_fb_helper_fini(fb_helper); - - if (shadow) - vfree(shadow); - else if (fb_helper->buffer) - drm_client_buffer_vunmap(fb_helper->buffer); - + vfree(shadow); drm_client_framebuffer_delete(fb_helper->buffer); - drm_client_release(&fb_helper->client); + drm_client_release(&fb_helper->client); drm_fb_helper_unprepare(fb_helper); kfree(fb_helper); } -static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) -{ - struct drm_fb_helper *fb_helper = info->par; - - if (drm_fbdev_use_shadow_fb(fb_helper)) - return fb_deferred_io_mmap(info, vma); - else if (fb_helper->dev->driver->gem_prime_mmap) - return fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma); - else - return -ENODEV; -} - -static bool drm_fbdev_use_iomem(struct fb_info *info) -{ - struct drm_fb_helper *fb_helper = info->par; - struct drm_client_buffer *buffer = fb_helper->buffer; - - return !drm_fbdev_use_shadow_fb(fb_helper) && buffer->map.is_iomem; -} - -static ssize_t drm_fbdev_fb_read(struct fb_info *info, char __user *buf, -size_t count, loff_t *ppos) -{ - ssize_t ret; - - if (drm_fbdev_use_iomem(info)) - ret = drm_fb_helper_cfb_read(info, buf, count, ppos); - else - ret = drm_fb_helper_sys_read(info, buf, count, ppos); - - return ret; -} - -static ssize_t drm_fbdev_fb_write(struct fb_info *info, const char __user *buf, - size_t count, loff_t *ppos) -{ - ssize_t ret; - - if (drm_fbdev_use_iomem(info)) - ret = drm_fb_helper_cfb_write(info, buf, count, ppos); - else - ret = drm_fb_helper_sys_write(info, buf, count, ppos); - - return ret; -} - -static void drm_fbdev_fb_fillrect(struct fb_info *info, - const struct fb_fillrect *rect) -{ - if (drm_fbdev_use_iomem(info)) - drm_fb_helper_cfb_fillrect(info, rect); - else - drm_fb_helper_sys_fillrect(info, rect); -} - -static void drm_fbdev_fb_copyarea(struct fb_info *info, - const struct fb_copyarea *area) -{ - if (drm_fbdev_use_iomem(info)) - drm_fb_helper_cfb_copyarea(info, area); - else - drm_fb_helper_sys_copyarea(info, area); -} - -static void drm_fbdev_fb_imageblit(struct fb_info *info, - const struct fb_image *image) -{ - if (drm_fbdev_use_iomem(info)) - drm_fb_helper_cfb_imageblit(info, image); - else - drm_fb_helper_sys_ima
Re: [PATCH v10 01/15] dma-buf/dma-fence: Add deadline awareness
On Wed, Mar 15, 2023 at 6:53 AM Jonas Ådahl wrote: > > On Fri, Mar 10, 2023 at 09:38:18AM -0800, Rob Clark wrote: > > On Fri, Mar 10, 2023 at 7:45 AM Jonas Ådahl wrote: > > > > > > On Wed, Mar 08, 2023 at 07:52:52AM -0800, Rob Clark wrote: > > > > From: Rob Clark > > > > > > > > Add a way to hint to the fence signaler of an upcoming deadline, such as > > > > vblank, which the fence waiter would prefer not to miss. This is to aid > > > > the fence signaler in making power management decisions, like boosting > > > > frequency as the deadline approaches and awareness of missing deadlines > > > > so that can be factored in to the frequency scaling. > > > > > > > > v2: Drop dma_fence::deadline and related logic to filter duplicate > > > > deadlines, to avoid increasing dma_fence size. The fence-context > > > > implementation will need similar logic to track deadlines of all > > > > the fences on the same timeline. [ckoenig] > > > > v3: Clarify locking wrt. set_deadline callback > > > > v4: Clarify in docs comment that this is a hint > > > > v5: Drop DMA_FENCE_FLAG_HAS_DEADLINE_BIT. > > > > v6: More docs > > > > v7: Fix typo, clarify past deadlines > > > > > > > > Signed-off-by: Rob Clark > > > > Reviewed-by: Christian König > > > > Acked-by: Pekka Paalanen > > > > Reviewed-by: Bagas Sanjaya > > > > --- > > > > > > Hi Rob! > > > > > > > Documentation/driver-api/dma-buf.rst | 6 +++ > > > > drivers/dma-buf/dma-fence.c | 59 > > > > include/linux/dma-fence.h| 22 +++ > > > > 3 files changed, 87 insertions(+) > > > > > > > > diff --git a/Documentation/driver-api/dma-buf.rst > > > > b/Documentation/driver-api/dma-buf.rst > > > > index 622b8156d212..183e480d8cea 100644 > > > > --- a/Documentation/driver-api/dma-buf.rst > > > > +++ b/Documentation/driver-api/dma-buf.rst > > > > @@ -164,6 +164,12 @@ DMA Fence Signalling Annotations > > > > .. kernel-doc:: drivers/dma-buf/dma-fence.c > > > > :doc: fence signalling annotation > > > > > > > > +DMA Fence Deadline Hints > > > > + > > > > + > > > > +.. kernel-doc:: drivers/dma-buf/dma-fence.c > > > > + :doc: deadline hints > > > > + > > > > DMA Fences Functions Reference > > > > ~~ > > > > > > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > > > > index 0de0482cd36e..f177c56269bb 100644 > > > > --- a/drivers/dma-buf/dma-fence.c > > > > +++ b/drivers/dma-buf/dma-fence.c > > > > @@ -912,6 +912,65 @@ dma_fence_wait_any_timeout(struct dma_fence > > > > **fences, uint32_t count, > > > > } > > > > EXPORT_SYMBOL(dma_fence_wait_any_timeout); > > > > > > > > +/** > > > > + * DOC: deadline hints > > > > + * > > > > + * In an ideal world, it would be possible to pipeline a workload > > > > sufficiently > > > > + * that a utilization based device frequency governor could arrive at > > > > a minimum > > > > + * frequency that meets the requirements of the use-case, in order to > > > > minimize > > > > + * power consumption. But in the real world there are many workloads > > > > which > > > > + * defy this ideal. For example, but not limited to: > > > > + * > > > > + * * Workloads that ping-pong between device and CPU, with alternating > > > > periods > > > > + * of CPU waiting for device, and device waiting on CPU. This can > > > > result in > > > > + * devfreq and cpufreq seeing idle time in their respective domains > > > > and in > > > > + * result reduce frequency. > > > > + * > > > > + * * Workloads that interact with a periodic time based deadline, such > > > > as double > > > > + * buffered GPU rendering vs vblank sync'd page flipping. In this > > > > scenario, > > > > + * missing a vblank deadline results in an *increase* in idle time > > > > on the GPU > > > > + * (since it has to wait an additional vblank period), sending a > > > > signal to > > > > + * the GPU's devfreq to reduce frequency, when in fact the opposite > > > > is what is > > > > + * needed. > > > > > > This is the use case I'd like to get some better understanding about how > > > this series intends to work, as the problematic scheduling behavior > > > triggered by missed deadlines has plagued compositing display servers > > > for a long time. > > > > > > I apologize, I'm not a GPU driver developer, nor an OpenGL driver > > > developer, so I will need some hand holding when it comes to > > > understanding exactly what piece of software is responsible for > > > communicating what piece of information. > > > > > > > + * > > > > + * To this end, deadline hint(s) can be set on a &dma_fence via > > > > &dma_fence_set_deadline. > > > > + * The deadline hint provides a way for the waiting driver, or > > > > userspace, to > > > > + * convey an appropriate sense of urgency to the signaling driver. > > > > + * > > > > + * A deadline hint is given in absolute ktime (CLOCK_MONOTONIC for > > > > userspace > > > > + * fac
Re: [PATCH 21/21] panel: startek-kd070fhfid015: add support of this display
Hi, On 15/03/2023 16:59, Alexandre Mergnat wrote: Hi Neil, Le jeu. 9 mars 2023 à 15:51, Neil Armstrong a écrit : + +#include Is this include needed ? Seems not, I remove it. + struct gpio_desc *enable_gpio; + struct gpio_desc *reset_gpio; + struct gpio_desc *dcdc_en_gpio; Isn't this "DCDC" a regulator ??? According to the panel datasheet, this is an input signal needed passing through TFT connector (PIN30): "Power IC supply enable, High active" Ack, perhaps the name isn't the right one, or specify something like panel-dcdc. + /* Interface setting, video mode */ + ret = mipi_dsi_generic_write(dsi, + (u8[]){DSI_REG_IS, 0x14, 0x08, 0x00, 0x22, 0x00}, 6); + if (ret < 0) { + dev_err(dev, "failed to set display interface setting: %d\n", + ret); + return ret; + } Use mipi_dsi_generic_write_seq(dsi, DSI_REG_IS, 0x14, 0x08, 0x00, 0x22, 0x00); instead, it's simpler and doesn't need explicit return check. "mipi_dsi_generic_write_seq" doesn't exist but I will add it below "mipi_dsi_dcs_write_seq" in drm_mipi_dsi.h It is now https://github.com/torvalds/linux/blob/master/include/drm/drm_mipi_dsi.h#L302 since a9015ce59320 I must keep the return check to jump out of "stk_panel_init" if something goes wrong, but I can drop the error print. The mipi_dsi_generic_write_seq & mipi_dsi_dcs_write_seq macros does the return check for free. +static int stk_panel_enable(struct drm_panel *panel) +{ + struct stk_panel *stk = to_stk_panel(panel); + + if (stk->enabled) + return 0; + + backlight_enable(stk->backlight); + + stk->enabled = true; + + return 0; +} Is this really needed ? the backlight core will enable it automatically. Ok, it's managed by drm_panel.c driver. Then I can also remove stuff from stk_panel_disable. Regards, Alex
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
On Wed, 15 Mar 2023 11:57:12 -0400 Steven Rostedt wrote: So I'm looking at the backtraces. > The WARN_ON triggered: > > [ 21.481449] mpls_gso: MPLS GSO support > [ 21.488795] IPI shorthand broadcast: enabled > [ 21.488873] [ cut here ] > [ 21.490101] [ cut here ] > > [ 21.491693] WARNING: CPU: 1 PID: 38 at drivers/gpu/drm/ttm/ttm_bo.c:332 > ttm_bo_release+0x2ac/0x2fc <<< Line of the added WARN_ON() This happened on CPU 1. > > [ 21.492940] refcount_t: underflow; use-after-free. > [ 21.492965] WARNING: CPU: 0 PID: 84 at lib/refcount.c:28 > refcount_warn_saturate+0xb6/0xfc This happened on CPU 0. > [ 21.496116] Modules linked in: > [ 21.497197] Modules linked in: > [ 21.500105] CPU: 1 PID: 38 Comm: kworker/1:1 Not tainted > 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 > [ 21.500789] CPU: 0 PID: 84 Comm: kworker/0:1H Not tainted > 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 > [ 21.501882] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > 1.16.0-debian-1.16.0-5 04/01/2014 > [ 21.503533] sched_clock: Marking stable (20788024762, > 714243692)->(22140778105, -638509651) > [ 21.504080] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > 1.16.0-debian-1.16.0-5 04/01/2014 > [ 21.504089] Workqueue: ttm ttm_bo_delayed_delete > [ 21.507196] Workqueue: events drm_fb_helper_damage_work > [ 21.509235] > [ 21.510291] registered taskstats version 1 > [ 21.510302] Running ring buffer tests... > [ 21.511792] > [ 21.513870] EIP: refcount_warn_saturate+0xb6/0xfc > [ 21.515261] EIP: ttm_bo_release+0x2ac/0x2fc > [ 21.516566] Code: 68 00 27 0c d8 e8 36 3b aa ff 0f 0b 58 c9 c3 90 80 3d 41 > c2 37 d8 00 75 8a c6 05 41 c2 37 d8 01 68 2c 27 0c d8 e8 16 3b aa ff <0f> 0b > 59 c9 c3 80 3d 3f c2 37 d8 00 0f 85 67 ff ff ff c6 05 3f c2 > [ 21.516998] Code: ff 8d b4 26 00 00 00 00 66 90 0f 0b 8b 43 10 85 c0 0f 84 > a1 fd ff ff 8d 76 00 0f 0b 8b 43 28 85 c0 0f 84 9c fd ff ff 8d 76 00 <0f> 0b > e9 92 fd ff ff 8d b4 26 00 00 00 00 66 90 c7 43 18 00 00 00 > [ 21.517905] EAX: 0026 EBX: c129d150 ECX: 0040 EDX: 0002 > [ 21.518987] EAX: d78c8550 EBX: c129d134 ECX: c129d134 EDX: 0001 > [ 21.519337] ESI: c129d0bc EDI: f6f91200 EBP: c2b8bf18 ESP: c2b8bf14 > [ 21.520617] ESI: c129d000 EDI: c126a7a0 EBP: c1839c24 ESP: c1839bec > [ 21.521546] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 > [ 21.526154] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 > [ 21.526162] CR0: 80050033 CR2: CR3: 18506000 CR4: 00150ef0 > [ 21.526166] Call Trace: > [ 21.526189] ? ww_mutex_unlock+0x3a/0x94 > [ 21.530300] CR0: 80050033 CR2: ff9ff000 CR3: 18506000 CR4: 00150ef0 > [ 21.531722] ? ttm_bo_cleanup_refs+0xc4/0x1e0 > [ 21.533114] Call Trace: > [ 21.534516] ttm_mem_evict_first+0x3d3/0x568 > [ 21.535901] ttm_bo_delayed_delete+0x9c/0xa4 > [ 21.537391] ? kfree+0x6b/0xdc > [ 21.538901] process_one_work+0x21a/0x484 > [ 21.540279] ? ttm_range_man_alloc+0xe0/0xec > [ 21.540854] worker_thread+0x14a/0x39c > [ 21.541714] ? ttm_range_man_fini_nocheck+0xe8/0xe8 > [ 21.543332] kthread+0xea/0x10c > [ 21.544301] ttm_bo_mem_space+0x1d0/0x1e4 > [ 21.544942] ? process_one_work+0x484/0x484 > [ 21.545887] ttm_bo_validate+0xc5/0x19c > [ 21.546986] ? kthread_complete_and_exit+0x1c/0x1c > [ 21.547680] ttm_bo_init_reserved+0x15e/0x1fc > [ 21.548716] ret_from_fork+0x1c/0x28 > [ 21.549650] qxl_bo_create+0x145/0x20c The qxl_bo_create() calls ttm_bo_init_reserved() as the object in question is about to be freed. I'm guessing what is happening here, is that an object was to be freed by the delayed_delete, and in the mean time, something else picked it up. What's protecting this from not being used again? -- Steve
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
On Wed, 15 Mar 2023 11:57:12 -0400 Steven Rostedt wrote: > The WARN_ON triggered: > > [ 21.481449] mpls_gso: MPLS GSO support > [ 21.488795] IPI shorthand broadcast: enabled > [ 21.488873] [ cut here ] > [ 21.490101] [ cut here ] > > [ 21.491693] WARNING: CPU: 1 PID: 38 at drivers/gpu/drm/ttm/ttm_bo.c:332 > ttm_bo_release+0x2ac/0x2fc <<< Line of the added WARN_ON() > > [ 21.492940] refcount_t: underflow; use-after-free. > [ 21.492965] WARNING: CPU: 0 PID: 84 at lib/refcount.c:28 > refcount_warn_saturate+0xb6/0xfc > [ 21.496116] Modules linked in: > [ 21.497197] Modules linked in: > [ 21.500105] CPU: 1 PID: 38 Comm: kworker/1:1 Not tainted > 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 > [ 21.500789] CPU: 0 PID: 84 Comm: kworker/0:1H Not tainted > 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 > [ 21.501882] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > 1.16.0-debian-1.16.0-5 04/01/2014 > [ 21.503533] sched_clock: Marking stable (20788024762, > 714243692)->(22140778105, -638509651) > [ 21.504080] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > 1.16.0-debian-1.16.0-5 04/01/2014 > [ 21.504089] Workqueue: ttm ttm_bo_delayed_delete > [ 21.507196] Workqueue: events drm_fb_helper_damage_work > [ 21.509235] > [ 21.510291] registered taskstats version 1 > [ 21.510302] Running ring buffer tests... > [ 21.511792] > [ 21.513870] EIP: refcount_warn_saturate+0xb6/0xfc > [ 21.515261] EIP: ttm_bo_release+0x2ac/0x2fc > [ 21.516566] Code: 68 00 27 0c d8 e8 36 3b aa ff 0f 0b 58 c9 c3 90 80 3d 41 > c2 37 d8 00 75 8a c6 05 41 c2 37 d8 01 68 2c 27 0c d8 e8 16 3b aa ff <0f> 0b > 59 c9 c3 80 3d 3f c2 37 d8 00 0f 85 67 ff ff ff c6 05 3f c2 > [ 21.516998] Code: ff 8d b4 26 00 00 00 00 66 90 0f 0b 8b 43 10 85 c0 0f 84 > a1 fd ff ff 8d 76 00 0f 0b 8b 43 28 85 c0 0f 84 9c fd ff ff 8d 76 00 <0f> 0b > e9 92 fd ff ff 8d b4 26 00 00 00 00 66 90 c7 43 18 00 00 00 > [ 21.517905] EAX: 0026 EBX: c129d150 ECX: 0040 EDX: 0002 > [ 21.518987] EAX: d78c8550 EBX: c129d134 ECX: c129d134 EDX: 0001 > [ 21.519337] ESI: c129d0bc EDI: f6f91200 EBP: c2b8bf18 ESP: c2b8bf14 > [ 21.520617] ESI: c129d000 EDI: c126a7a0 EBP: c1839c24 ESP: c1839bec > [ 21.521546] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 > [ 21.526154] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 > [ 21.526162] CR0: 80050033 CR2: CR3: 18506000 CR4: 00150ef0 > [ 21.526166] Call Trace: > [ 21.526189] ? ww_mutex_unlock+0x3a/0x94 > [ 21.530300] CR0: 80050033 CR2: ff9ff000 CR3: 18506000 CR4: 00150ef0 > [ 21.531722] ? ttm_bo_cleanup_refs+0xc4/0x1e0 > [ 21.533114] Call Trace: > [ 21.534516] ttm_mem_evict_first+0x3d3/0x568 > [ 21.535901] ttm_bo_delayed_delete+0x9c/0xa4 > [ 21.537391] ? kfree+0x6b/0xdc > [ 21.538901] process_one_work+0x21a/0x484 > [ 21.540279] ? ttm_range_man_alloc+0xe0/0xec > [ 21.540854] worker_thread+0x14a/0x39c > [ 21.541714] ? ttm_range_man_fini_nocheck+0xe8/0xe8 > [ 21.543332] kthread+0xea/0x10c So I triggered it again, and the same backtrace is there. > [ 21.544301] ttm_bo_mem_space+0x1d0/0x1e4 It looks like the object is being reserved before it's fully removed. And it's somewhere in this tty_bo_mem_space() (which comes from the qxl_bo_create()). I don't know this code at all, nor do I have any idea of what it's trying to do. All I know is that this is triggering often (not always), and it has to do with some race. Now my config has lots of debugging enabled, which slows down the system quite a bit. This also happens to open up race windows. Just because your testing doesn't trigger it, doesn't mean that the race doesn't exist. It's just likely to be very hard to hit. > [ 21.544942] ? process_one_work+0x484/0x484 > [ 21.545887] ttm_bo_validate+0xc5/0x19c > [ 21.546986] ? kthread_complete_and_exit+0x1c/0x1c > [ 21.547680] ttm_bo_init_reserved+0x15e/0x1fc > [ 21.548716] ret_from_fork+0x1c/0x28 > [ 21.549650] qxl_bo_create+0x145/0x20c Here's the latest backtrace: [ 170.817449] [ cut here ] [ 170.817455] [ cut here ] [ 170.818210] refcount_t: underflow; use-after-free. [ 170.818228] WARNING: CPU: 0 PID: 267 at lib/refcount.c:28 refcount_warn_saturate+0xb6/0xfc [ 170.819352] WARNING: CPU: 3 PID: 2382 at drivers/gpu/drm/ttm/ttm_bo.c:332 ttm_bo_release+0x278/0x2c8 [ 170.820124] Modules linked in: [ 170.822127] Modules linked in: [ 170.823829] [ 170.823832] CPU: 0 PID: 267 Comm: kworker/0:10H Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #998 [ 170.824610] [ 170.825121] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 170.825124] Workqueue: ttm ttm_bo_delayed_delete [ 170.825498] CPU: 3 PID: 2382 Comm: kworker/3:3 Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #998 [ 170.826996] [ 17
[PATCH] drm/i915/selftests: keep same cache settings as timeline
From: Fei Yang On MTL, objects allocated through i915_gem_object_create_internal() are mapped as uncached in GPU by default because HAS_LLC is false. However in the live_hwsp_read selftest these watcher objects are mapped as WB on CPU side. The conseqence is that the updates done by the GPU are not immediately visible to CPU, thus the selftest is randomly failing due to the stale data in CPU cache. Solution can be either setting WC for CPU + UC for GPU, or WB for CPU + 1-way coherent WB for GPU. To keep the consistency, let's simply inherit the same cache settings from the timeline, which is WB for CPU + 1-way coherent WB for GPU, because this test is supposed to emulate the behavior of the timeline anyway. Signed-off-by: Fei Yang --- drivers/gpu/drm/i915/gt/selftest_timeline.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c b/drivers/gpu/drm/i915/gt/selftest_timeline.c index 522d0190509c..631aaed9bc3d 100644 --- a/drivers/gpu/drm/i915/gt/selftest_timeline.c +++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c @@ -825,7 +825,8 @@ static bool cmp_gte(u32 a, u32 b) return a >= b; } -static int setup_watcher(struct hwsp_watcher *w, struct intel_gt *gt) +static int setup_watcher(struct hwsp_watcher *w, struct intel_gt *gt, +struct intel_timeline *tl) { struct drm_i915_gem_object *obj; struct i915_vma *vma; @@ -834,7 +835,10 @@ static int setup_watcher(struct hwsp_watcher *w, struct intel_gt *gt) if (IS_ERR(obj)) return PTR_ERR(obj); - w->map = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); + /* keep the same cache settings as timeline */ + i915_gem_object_set_cache_coherency(obj, tl->hwsp_ggtt->obj->cache_level); + w->map = i915_gem_object_pin_map_unlocked(obj, + page_unmask_bits(tl->hwsp_ggtt->obj->mm.mapping)); if (IS_ERR(w->map)) { i915_gem_object_put(obj); return PTR_ERR(w->map); @@ -1004,8 +1008,10 @@ static int live_hwsp_read(void *arg) if (!tl->has_initial_breadcrumb) goto out_free; + selftest_tl_pin(tl); + for (i = 0; i < ARRAY_SIZE(watcher); i++) { - err = setup_watcher(&watcher[i], gt); + err = setup_watcher(&watcher[i], gt, tl); if (err) goto out; } @@ -1160,6 +1166,8 @@ static int live_hwsp_read(void *arg) for (i = 0; i < ARRAY_SIZE(watcher); i++) cleanup_watcher(&watcher[i]); + intel_timeline_unpin(tl); + if (igt_flush_test(gt->i915)) err = -EIO; -- 2.25.1
Re: [PATCH 1/2] docs: process: allow Closes tags with links
On Wed, Mar 15, 2023 at 06:44:56PM +0100, Matthieu Baerts wrote: > Note that thanks to this "Closes" tag, the mentioned bug trackers can > also locate where a patch has been applied in different branches and > repositories. If only the "Link" tag is used, the tracking can also be > done but the ticket will not be closed and a manual operation will be > needed. We will soon gain this ability on bugzilla.kernel.org as one of the features of the bugbot integration tool (which is still WIP). So, if it helps, I support making this a recognized trailer. Acked-by: Konstantin Ryabitsev -K
Re: [PATCH 1/2] docs: process: allow Closes tags with links
Matthieu Baerts writes: > +In the same category as linking web pages, a special tag is also used to > close > +issues but only when the mentioned ticketing system can do this operation > +automatically:: > + > +Closes: https://example.com/issues/1234 > + > +Please use this 'Closes:' tag only if it helps managing issues thanks to > +automations. If not, pick the 'Link:' one. So if there is a consensus for this, I can certainly apply the patch. I do think, though, that if we accept this tag, we should ask that it only be used for *public* trackers. A bunch of tags referring to internal trackers and such aren't going to be all that helpful. jon
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
Am 15.03.23 um 18:54 schrieb Steven Rostedt: On Wed, 15 Mar 2023 11:57:12 -0400 Steven Rostedt wrote: The WARN_ON triggered: [ 21.481449] mpls_gso: MPLS GSO support [ 21.488795] IPI shorthand broadcast: enabled [ 21.488873] [ cut here ] [ 21.490101] [ cut here ] [ 21.491693] WARNING: CPU: 1 PID: 38 at drivers/gpu/drm/ttm/ttm_bo.c:332 ttm_bo_release+0x2ac/0x2fc <<< Line of the added WARN_ON() [ 21.492940] refcount_t: underflow; use-after-free. [ 21.492965] WARNING: CPU: 0 PID: 84 at lib/refcount.c:28 refcount_warn_saturate+0xb6/0xfc [ 21.496116] Modules linked in: [ 21.497197] Modules linked in: The problem here is that two backtraces mix together. So it's pretty hard to figure out what's going on. [ 21.500105] CPU: 1 PID: 38 Comm: kworker/1:1 Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 [ 21.500789] CPU: 0 PID: 84 Comm: kworker/0:1H Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 [ 21.501882] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 21.503533] sched_clock: Marking stable (20788024762, 714243692)->(22140778105, -638509651) [ 21.504080] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 21.504089] Workqueue: ttm ttm_bo_delayed_delete [ 21.507196] Workqueue: events drm_fb_helper_damage_work [ 21.509235] [ 21.510291] registered taskstats version 1 [ 21.510302] Running ring buffer tests... [ 21.511792] [ 21.513870] EIP: refcount_warn_saturate+0xb6/0xfc [ 21.515261] EIP: ttm_bo_release+0x2ac/0x2fc [ 21.516566] Code: 68 00 27 0c d8 e8 36 3b aa ff 0f 0b 58 c9 c3 90 80 3d 41 c2 37 d8 00 75 8a c6 05 41 c2 37 d8 01 68 2c 27 0c d8 e8 16 3b aa ff <0f> 0b 59 c9 c3 80 3d 3f c2 37 d8 00 0f 85 67 ff ff ff c6 05 3f c2 [ 21.516998] Code: ff 8d b4 26 00 00 00 00 66 90 0f 0b 8b 43 10 85 c0 0f 84 a1 fd ff ff 8d 76 00 0f 0b 8b 43 28 85 c0 0f 84 9c fd ff ff 8d 76 00 <0f> 0b e9 92 fd ff ff 8d b4 26 00 00 00 00 66 90 c7 43 18 00 00 00 [ 21.517905] EAX: 0026 EBX: c129d150 ECX: 0040 EDX: 0002 [ 21.518987] EAX: d78c8550 EBX: c129d134 ECX: c129d134 EDX: 0001 [ 21.519337] ESI: c129d0bc EDI: f6f91200 EBP: c2b8bf18 ESP: c2b8bf14 [ 21.520617] ESI: c129d000 EDI: c126a7a0 EBP: c1839c24 ESP: c1839bec [ 21.521546] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 [ 21.526154] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 [ 21.526162] CR0: 80050033 CR2: CR3: 18506000 CR4: 00150ef0 [ 21.526166] Call Trace: [ 21.526189] ? ww_mutex_unlock+0x3a/0x94 [ 21.530300] CR0: 80050033 CR2: ff9ff000 CR3: 18506000 CR4: 00150ef0 [ 21.531722] ? ttm_bo_cleanup_refs+0xc4/0x1e0 [ 21.533114] Call Trace: [ 21.534516] ttm_mem_evict_first+0x3d3/0x568 [ 21.535901] ttm_bo_delayed_delete+0x9c/0xa4 [ 21.537391] ? kfree+0x6b/0xdc [ 21.538901] process_one_work+0x21a/0x484 [ 21.540279] ? ttm_range_man_alloc+0xe0/0xec [ 21.540854] worker_thread+0x14a/0x39c [ 21.541714] ? ttm_range_man_fini_nocheck+0xe8/0xe8 [ 21.543332] kthread+0xea/0x10c So I triggered it again, and the same backtrace is there. [ 21.544301] ttm_bo_mem_space+0x1d0/0x1e4 It looks like the object is being reserved before it's fully removed. And it's somewhere in this tty_bo_mem_space() (which comes from the qxl_bo_create()). I don't know this code at all, nor do I have any idea of what it's trying to do. All I know is that this is triggering often (not always), and it has to do with some race. Now my config has lots of debugging enabled, which slows down the system quite a bit. This also happens to open up race windows. Just because your testing doesn't trigger it, doesn't mean that the race doesn't exist. It's just likely to be very hard to hit. [ 21.544942] ? process_one_work+0x484/0x484 [ 21.545887] ttm_bo_validate+0xc5/0x19c [ 21.546986] ? kthread_complete_and_exit+0x1c/0x1c [ 21.547680] ttm_bo_init_reserved+0x15e/0x1fc [ 21.548716] ret_from_fork+0x1c/0x28 [ 21.549650] qxl_bo_create+0x145/0x20c Here's the latest backtrace: [ 170.817449] [ cut here ] [ 170.817455] [ cut here ] [ 170.818210] refcount_t: underflow; use-after-free. [ 170.818228] WARNING: CPU: 0 PID: 267 at lib/refcount.c:28 refcount_warn_saturate+0xb6/0xfc [ 170.819352] WARNING: CPU: 3 PID: 2382 at drivers/gpu/drm/ttm/ttm_bo.c:332 ttm_bo_release+0x278/0x2c8 [ 170.820124] Modules linked in: [ 170.822127] Modules linked in: [ 170.823829] [ 170.823832] CPU: 0 PID: 267 Comm: kworker/0:10H Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #998 [ 170.824610] [ 170.825121] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 170.825124] Workqueue: ttm ttm_bo_delayed_delete [ 170.825498] CPU: 3 PID: 2382 Comm: kworker/3:3 Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
Am 15.03.23 um 18:31 schrieb Steven Rostedt: On Wed, 15 Mar 2023 11:57:12 -0400 Steven Rostedt wrote: So I'm looking at the backtraces. The WARN_ON triggered: [ 21.481449] mpls_gso: MPLS GSO support [ 21.488795] IPI shorthand broadcast: enabled [ 21.488873] [ cut here ] [ 21.490101] [ cut here ] [ 21.491693] WARNING: CPU: 1 PID: 38 at drivers/gpu/drm/ttm/ttm_bo.c:332 ttm_bo_release+0x2ac/0x2fc <<< Line of the added WARN_ON() This happened on CPU 1. [ 21.492940] refcount_t: underflow; use-after-free. [ 21.492965] WARNING: CPU: 0 PID: 84 at lib/refcount.c:28 refcount_warn_saturate+0xb6/0xfc This happened on CPU 0. [ 21.496116] Modules linked in: [ 21.497197] Modules linked in: [ 21.500105] CPU: 1 PID: 38 Comm: kworker/1:1 Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 [ 21.500789] CPU: 0 PID: 84 Comm: kworker/0:1H Not tainted 6.3.0-rc2-test-00047-g6015b1aca1a2-dirty #993 [ 21.501882] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 21.503533] sched_clock: Marking stable (20788024762, 714243692)->(22140778105, -638509651) [ 21.504080] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 21.504089] Workqueue: ttm ttm_bo_delayed_delete [ 21.507196] Workqueue: events drm_fb_helper_damage_work [ 21.509235] [ 21.510291] registered taskstats version 1 [ 21.510302] Running ring buffer tests... [ 21.511792] [ 21.513870] EIP: refcount_warn_saturate+0xb6/0xfc [ 21.515261] EIP: ttm_bo_release+0x2ac/0x2fc [ 21.516566] Code: 68 00 27 0c d8 e8 36 3b aa ff 0f 0b 58 c9 c3 90 80 3d 41 c2 37 d8 00 75 8a c6 05 41 c2 37 d8 01 68 2c 27 0c d8 e8 16 3b aa ff <0f> 0b 59 c9 c3 80 3d 3f c2 37 d8 00 0f 85 67 ff ff ff c6 05 3f c2 [ 21.516998] Code: ff 8d b4 26 00 00 00 00 66 90 0f 0b 8b 43 10 85 c0 0f 84 a1 fd ff ff 8d 76 00 0f 0b 8b 43 28 85 c0 0f 84 9c fd ff ff 8d 76 00 <0f> 0b e9 92 fd ff ff 8d b4 26 00 00 00 00 66 90 c7 43 18 00 00 00 [ 21.517905] EAX: 0026 EBX: c129d150 ECX: 0040 EDX: 0002 [ 21.518987] EAX: d78c8550 EBX: c129d134 ECX: c129d134 EDX: 0001 [ 21.519337] ESI: c129d0bc EDI: f6f91200 EBP: c2b8bf18 ESP: c2b8bf14 [ 21.520617] ESI: c129d000 EDI: c126a7a0 EBP: c1839c24 ESP: c1839bec [ 21.521546] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 [ 21.526154] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 [ 21.526162] CR0: 80050033 CR2: CR3: 18506000 CR4: 00150ef0 [ 21.526166] Call Trace: [ 21.526189] ? ww_mutex_unlock+0x3a/0x94 [ 21.530300] CR0: 80050033 CR2: ff9ff000 CR3: 18506000 CR4: 00150ef0 [ 21.531722] ? ttm_bo_cleanup_refs+0xc4/0x1e0 [ 21.533114] Call Trace: [ 21.534516] ttm_mem_evict_first+0x3d3/0x568 [ 21.535901] ttm_bo_delayed_delete+0x9c/0xa4 [ 21.537391] ? kfree+0x6b/0xdc [ 21.538901] process_one_work+0x21a/0x484 [ 21.540279] ? ttm_range_man_alloc+0xe0/0xec [ 21.540854] worker_thread+0x14a/0x39c [ 21.541714] ? ttm_range_man_fini_nocheck+0xe8/0xe8 [ 21.543332] kthread+0xea/0x10c [ 21.544301] ttm_bo_mem_space+0x1d0/0x1e4 [ 21.544942] ? process_one_work+0x484/0x484 [ 21.545887] ttm_bo_validate+0xc5/0x19c [ 21.546986] ? kthread_complete_and_exit+0x1c/0x1c [ 21.547680] ttm_bo_init_reserved+0x15e/0x1fc [ 21.548716] ret_from_fork+0x1c/0x28 [ 21.549650] qxl_bo_create+0x145/0x20c The qxl_bo_create() calls ttm_bo_init_reserved() as the object in question is about to be freed. I'm guessing what is happening here, is that an object was to be freed by the delayed_delete, and in the mean time, something else picked it up. What's protecting this from not being used again? The reference count. This is pretty clearly an unbalanced reference counting issue. It's just that previously you wouldn't notice it much because we were just silently removing the BO from the LRU list without checking if it was already removed (and so just damaging a bit of memory). While now we get tons of errors because the delayed worker actually runs no matter if the BO is already freed or not. Christian. -- Steve
Re: [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
Am 08.03.23 um 13:43 schrieb Steven Rostedt: On Wed, 8 Mar 2023 07:17:38 +0100 Christian König wrote: What test case/environment do you run to trigger this? I'm running a 32bit x86 qemu instance. Attached is the config. The libvirt xml file is here: https://rostedt.org/vm-images/tracetest-32.xml and the VM image itself is here: https://rostedt.org/vm-images/tracetest-32.qcow2.bz2 I've started to download that, but it will take about an hour. So I tried to avoid that for now. But looks like there isn't any other way to reproduce this, the code seems to work with both amdgpu and radeon. My suspicion is that we just have a reference count issue in qxl or ttm which was never noticed because it didn't caused any problems (except for a minor memory corruption). Now you get a rain of warnings because we try to grab the lock in the delete worker. Christian. It happened again in another test (it's not 100% reproducible). [ 23.234838] [ cut here ] [ 23.236391] DEBUG_LOCKS_WARN_ON(lock->magic != lock) [ 23.236429] WARNING: CPU: 0 PID: 61 at kernel/locking/mutex.c:582 __ww_mutex_lock.constprop.0+0x566/0xfec [ 23.240990] Modules linked in: [ 23.242368] CPU: 0 PID: 61 Comm: kworker/0:1H Not tainted 6.3.0-rc1-test-1-ga98bd42762ed-dirty #972 [ 23.245106] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 23.247900] Workqueue: ttm ttm_bo_delayed_delete [ 23.249642] EIP: __ww_mutex_lock.constprop.0+0x566/0xfec [ 23.251563] Code: e8 2b 5a 95 ff 85 c0 0f 84 25 fb ff ff 8b 0d 18 71 3b c8 85 c9 0f 85 17 fb ff ff 68 c0 58 07 c8 68 07 77 05 c8 e8 e6 0a 40 ff <0f> 0b 58 5a e9 ff fa ff ff e8 f8 59 95 ff 85 c0 74 0e 8b 0d 18 71 [ 23.256901] EAX: 0028 EBX: ECX: c1847dd8 EDX: 0002 [ 23.258849] ESI: EDI: c12958bc EBP: c1847f00 ESP: c1847eac [ 23.260786] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 [ 23.262840] CR0: 80050033 CR2: ffbff000 CR3: 0850e000 CR4: 00150ef0 [ 23.264781] Call Trace: [ 23.265899] ? lock_is_held_type+0xbe/0x10c [ 23.267434] ? ttm_bo_delayed_delete+0x30/0x94 [ 23.268971] ww_mutex_lock+0x32/0x94 [ 23.270327] ttm_bo_delayed_delete+0x30/0x94 [ 23.271818] process_one_work+0x21a/0x538 [ 23.273242] worker_thread+0x146/0x398 [ 23.274616] kthread+0xea/0x10c [ 23.275859] ? process_one_work+0x538/0x538 [ 23.277312] ? kthread_complete_and_exit+0x1c/0x1c [ 23.278899] ret_from_fork+0x1c/0x28 [ 23.280223] irq event stamp: 33 [ 23.281440] hardirqs last enabled at (33): [] _raw_spin_unlock_irqrestore+0x2d/0x58 [ 23.283860] hardirqs last disabled at (32): [] kvfree_call_rcu+0x155/0x2ec [ 23.286066] softirqs last enabled at (0): [] copy_process+0x989/0x2368 [ 23.288220] softirqs last disabled at (0): [<>] 0x0 [ 23.289952] ---[ end trace ]--- [ 23.291501] [ cut here ] [ 23.293027] refcount_t: underflow; use-after-free. [ 23.294644] WARNING: CPU: 0 PID: 61 at lib/refcount.c:28 refcount_warn_saturate+0xb6/0xfc [ 23.296959] Modules linked in: [ 23.298168] CPU: 0 PID: 61 Comm: kworker/0:1H Tainted: GW 6.3.0-rc1-test-1-ga98bd42762ed-dirty #972 [ 23.301073] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 23.303642] Workqueue: ttm ttm_bo_delayed_delete [ 23.305190] EIP: refcount_warn_saturate+0xb6/0xfc [ 23.306767] Code: 68 70 e1 0c c8 e8 f6 d6 a9 ff 0f 0b 58 c9 c3 90 80 3d 8a 78 38 c8 00 75 8a c6 05 8a 78 38 c8 01 68 9c e1 0c c8 e8 d6 d6 a9 ff <0f> 0b 59 c9 c3 80 3d 88 78 38 c8 00 0f 85 67 ff ff ff c6 05 88 78 [ 23.311935] EAX: 0026 EBX: c1295950 ECX: c1847e40 EDX: 0002 [ 23.313884] ESI: c12958bc EDI: f7591100 EBP: c1847f18 ESP: c1847f14 [ 23.315840] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010246 [ 23.317887] CR0: 80050033 CR2: ffbff000 CR3: 0850e000 CR4: 00150ef0 [ 23.319859] Call Trace: [ 23.320978] ttm_bo_delayed_delete+0x8c/0x94 [ 23.322492] process_one_work+0x21a/0x538 [ 23.323959] worker_thread+0x146/0x398 [ 23.325353] kthread+0xea/0x10c [ 23.326609] ? process_one_work+0x538/0x538 [ 23.328081] ? kthread_complete_and_exit+0x1c/0x1c [ 23.329683] ret_from_fork+0x1c/0x28 [ 23.331011] irq event stamp: 33 [ 23.332251] hardirqs last enabled at (33): [] _raw_spin_unlock_irqrestore+0x2d/0x58 [ 23.334334] hardirqs last disabled at (32): [] kvfree_call_rcu+0x155/0x2ec [ 23.336176] softirqs last enabled at (0): [] copy_process+0x989/0x2368 [ 23.337904] softirqs last disabled at (0): [<>] 0x0 [ 23.339313] ---[ end trace ]--- -- Steve
Re: [PATCH 1/2] docs: process: allow Closes tags with links
On Wed, 15 Mar 2023 18:44:56 +0100 Matthieu Baerts wrote: > +Closes: https://example.com/issues/1234 I (and a few others) have been using "Addresses:" on occasion. I think "Addresses:" is a bit more general. And more humble - "I tried to fix it", not "there, I fixed it". But whatever - both are good.
Re: [Intel-gfx] [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
On Wed, 15 Mar 2023 at 18:41, Christian König wrote: > > Am 08.03.23 um 13:43 schrieb Steven Rostedt: > > On Wed, 8 Mar 2023 07:17:38 +0100 > > Christian König wrote: > > > >> What test case/environment do you run to trigger this? > > I'm running a 32bit x86 qemu instance. Attached is the config. > > > > The libvirt xml file is here: https://rostedt.org/vm-images/tracetest-32.xml > > and the VM image itself is here: > > https://rostedt.org/vm-images/tracetest-32.qcow2.bz2 > > I've started to download that, but it will take about an hour. So I > tried to avoid that for now. > > But looks like there isn't any other way to reproduce this, the code > seems to work with both amdgpu and radeon. > > My suspicion is that we just have a reference count issue in qxl or ttm > which was never noticed because it didn't caused any problems (except > for a minor memory corruption). Why does ttm_bo_cleanup_refs() do a bo_put() at the end? It doesn't make sense to me. Say if the BO is in the process of being delay freed (bo->deleted = true), and we just did the kref_init() in ttm_bo_release(), it might drop that ref hitting ttm_bo_release() yet again, this time doing the actual bo->destroy(), which frees the object. The worker then fires at some later point calling ttm_bo_delayed_delete(), but the BO has already been freed. > > Now you get a rain of warnings because we try to grab the lock in the > delete worker. > > Christian. > > > > > It happened again in another test (it's not 100% reproducible). > > > > [ 23.234838] [ cut here ] > > [ 23.236391] DEBUG_LOCKS_WARN_ON(lock->magic != lock) > > [ 23.236429] WARNING: CPU: 0 PID: 61 at kernel/locking/mutex.c:582 > > __ww_mutex_lock.constprop.0+0x566/0xfec > > [ 23.240990] Modules linked in: > > [ 23.242368] CPU: 0 PID: 61 Comm: kworker/0:1H Not tainted > > 6.3.0-rc1-test-1-ga98bd42762ed-dirty #972 > > [ 23.245106] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > > 1.16.0-debian-1.16.0-5 04/01/2014 > > [ 23.247900] Workqueue: ttm ttm_bo_delayed_delete > > [ 23.249642] EIP: __ww_mutex_lock.constprop.0+0x566/0xfec > > [ 23.251563] Code: e8 2b 5a 95 ff 85 c0 0f 84 25 fb ff ff 8b 0d 18 71 3b > > c8 85 c9 0f 85 17 fb ff ff 68 c0 58 07 c8 68 07 77 05 c8 e8 e6 0a 40 ff > > <0f> 0b 58 5a e9 ff fa ff ff e8 f8 59 95 ff 85 c0 74 0e 8b 0d 18 71 > > [ 23.256901] EAX: 0028 EBX: ECX: c1847dd8 EDX: 0002 > > [ 23.258849] ESI: EDI: c12958bc EBP: c1847f00 ESP: c1847eac > > [ 23.260786] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 > > [ 23.262840] CR0: 80050033 CR2: ffbff000 CR3: 0850e000 CR4: 00150ef0 > > [ 23.264781] Call Trace: > > [ 23.265899] ? lock_is_held_type+0xbe/0x10c > > [ 23.267434] ? ttm_bo_delayed_delete+0x30/0x94 > > [ 23.268971] ww_mutex_lock+0x32/0x94 > > [ 23.270327] ttm_bo_delayed_delete+0x30/0x94 > > [ 23.271818] process_one_work+0x21a/0x538 > > [ 23.273242] worker_thread+0x146/0x398 > > [ 23.274616] kthread+0xea/0x10c > > [ 23.275859] ? process_one_work+0x538/0x538 > > [ 23.277312] ? kthread_complete_and_exit+0x1c/0x1c > > [ 23.278899] ret_from_fork+0x1c/0x28 > > [ 23.280223] irq event stamp: 33 > > [ 23.281440] hardirqs last enabled at (33): [] > > _raw_spin_unlock_irqrestore+0x2d/0x58 > > [ 23.283860] hardirqs last disabled at (32): [] > > kvfree_call_rcu+0x155/0x2ec > > [ 23.286066] softirqs last enabled at (0): [] > > copy_process+0x989/0x2368 > > [ 23.288220] softirqs last disabled at (0): [<>] 0x0 > > [ 23.289952] ---[ end trace ]--- > > [ 23.291501] [ cut here ] > > [ 23.293027] refcount_t: underflow; use-after-free. > > [ 23.294644] WARNING: CPU: 0 PID: 61 at lib/refcount.c:28 > > refcount_warn_saturate+0xb6/0xfc > > [ 23.296959] Modules linked in: > > [ 23.298168] CPU: 0 PID: 61 Comm: kworker/0:1H Tainted: GW > > 6.3.0-rc1-test-1-ga98bd42762ed-dirty #972 > > [ 23.301073] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > > 1.16.0-debian-1.16.0-5 04/01/2014 > > [ 23.303642] Workqueue: ttm ttm_bo_delayed_delete > > [ 23.305190] EIP: refcount_warn_saturate+0xb6/0xfc > > [ 23.306767] Code: 68 70 e1 0c c8 e8 f6 d6 a9 ff 0f 0b 58 c9 c3 90 80 3d > > 8a 78 38 c8 00 75 8a c6 05 8a 78 38 c8 01 68 9c e1 0c c8 e8 d6 d6 a9 ff > > <0f> 0b 59 c9 c3 80 3d 88 78 38 c8 00 0f 85 67 ff ff ff c6 05 88 78 > > [ 23.311935] EAX: 0026 EBX: c1295950 ECX: c1847e40 EDX: 0002 > > [ 23.313884] ESI: c12958bc EDI: f7591100 EBP: c1847f18 ESP: c1847f14 > > [ 23.315840] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010246 > > [ 23.317887] CR0: 80050033 CR2: ffbff000 CR3: 0850e000 CR4: 00150ef0 > > [ 23.319859] Call Trace: > > [ 23.320978] ttm_bo_delayed_delete+0x8c/0x94 > > [ 23.322492] process_one_work+0x21a/0x538 > > [ 23.323959] worker_thread+0x146/0x398 > > [ 23.325353] kthread+0xea/0x10c > > [
Re: [Intel-gfx] [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
Am 15.03.23 um 20:15 schrieb Matthew Auld: On Wed, 15 Mar 2023 at 18:41, Christian König wrote: Am 08.03.23 um 13:43 schrieb Steven Rostedt: On Wed, 8 Mar 2023 07:17:38 +0100 Christian König wrote: What test case/environment do you run to trigger this? I'm running a 32bit x86 qemu instance. Attached is the config. The libvirt xml file is here: https://rostedt.org/vm-images/tracetest-32.xml and the VM image itself is here: https://rostedt.org/vm-images/tracetest-32.qcow2.bz2 I've started to download that, but it will take about an hour. So I tried to avoid that for now. But looks like there isn't any other way to reproduce this, the code seems to work with both amdgpu and radeon. My suspicion is that we just have a reference count issue in qxl or ttm which was never noticed because it didn't caused any problems (except for a minor memory corruption). Why does ttm_bo_cleanup_refs() do a bo_put() at the end? Yeah, that's the bug. I own you a beer! In the old model we had an extra reference while the BOs where on the deleted list and I've forgot to remove this put here. Steven please try the attached patch. Thanks, Christian. It doesn't make sense to me. Say if the BO is in the process of being delay freed (bo->deleted = true), and we just did the kref_init() in ttm_bo_release(), it might drop that ref hitting ttm_bo_release() yet again, this time doing the actual bo->destroy(), which frees the object. The worker then fires at some later point calling ttm_bo_delayed_delete(), but the BO has already been freed. Now you get a rain of warnings because we try to grab the lock in the delete worker. Christian. It happened again in another test (it's not 100% reproducible). [ 23.234838] [ cut here ] [ 23.236391] DEBUG_LOCKS_WARN_ON(lock->magic != lock) [ 23.236429] WARNING: CPU: 0 PID: 61 at kernel/locking/mutex.c:582 __ww_mutex_lock.constprop.0+0x566/0xfec [ 23.240990] Modules linked in: [ 23.242368] CPU: 0 PID: 61 Comm: kworker/0:1H Not tainted 6.3.0-rc1-test-1-ga98bd42762ed-dirty #972 [ 23.245106] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 23.247900] Workqueue: ttm ttm_bo_delayed_delete [ 23.249642] EIP: __ww_mutex_lock.constprop.0+0x566/0xfec [ 23.251563] Code: e8 2b 5a 95 ff 85 c0 0f 84 25 fb ff ff 8b 0d 18 71 3b c8 85 c9 0f 85 17 fb ff ff 68 c0 58 07 c8 68 07 77 05 c8 e8 e6 0a 40 ff <0f> 0b 58 5a e9 ff fa ff ff e8 f8 59 95 ff 85 c0 74 0e 8b 0d 18 71 [ 23.256901] EAX: 0028 EBX: ECX: c1847dd8 EDX: 0002 [ 23.258849] ESI: EDI: c12958bc EBP: c1847f00 ESP: c1847eac [ 23.260786] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010286 [ 23.262840] CR0: 80050033 CR2: ffbff000 CR3: 0850e000 CR4: 00150ef0 [ 23.264781] Call Trace: [ 23.265899] ? lock_is_held_type+0xbe/0x10c [ 23.267434] ? ttm_bo_delayed_delete+0x30/0x94 [ 23.268971] ww_mutex_lock+0x32/0x94 [ 23.270327] ttm_bo_delayed_delete+0x30/0x94 [ 23.271818] process_one_work+0x21a/0x538 [ 23.273242] worker_thread+0x146/0x398 [ 23.274616] kthread+0xea/0x10c [ 23.275859] ? process_one_work+0x538/0x538 [ 23.277312] ? kthread_complete_and_exit+0x1c/0x1c [ 23.278899] ret_from_fork+0x1c/0x28 [ 23.280223] irq event stamp: 33 [ 23.281440] hardirqs last enabled at (33): [] _raw_spin_unlock_irqrestore+0x2d/0x58 [ 23.283860] hardirqs last disabled at (32): [] kvfree_call_rcu+0x155/0x2ec [ 23.286066] softirqs last enabled at (0): [] copy_process+0x989/0x2368 [ 23.288220] softirqs last disabled at (0): [<>] 0x0 [ 23.289952] ---[ end trace ]--- [ 23.291501] [ cut here ] [ 23.293027] refcount_t: underflow; use-after-free. [ 23.294644] WARNING: CPU: 0 PID: 61 at lib/refcount.c:28 refcount_warn_saturate+0xb6/0xfc [ 23.296959] Modules linked in: [ 23.298168] CPU: 0 PID: 61 Comm: kworker/0:1H Tainted: GW 6.3.0-rc1-test-1-ga98bd42762ed-dirty #972 [ 23.301073] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 [ 23.303642] Workqueue: ttm ttm_bo_delayed_delete [ 23.305190] EIP: refcount_warn_saturate+0xb6/0xfc [ 23.306767] Code: 68 70 e1 0c c8 e8 f6 d6 a9 ff 0f 0b 58 c9 c3 90 80 3d 8a 78 38 c8 00 75 8a c6 05 8a 78 38 c8 01 68 9c e1 0c c8 e8 d6 d6 a9 ff <0f> 0b 59 c9 c3 80 3d 88 78 38 c8 00 0f 85 67 ff ff ff c6 05 88 78 [ 23.311935] EAX: 0026 EBX: c1295950 ECX: c1847e40 EDX: 0002 [ 23.313884] ESI: c12958bc EDI: f7591100 EBP: c1847f18 ESP: c1847f14 [ 23.315840] DS: 007b ES: 007b FS: 00d8 GS: SS: 0068 EFLAGS: 00010246 [ 23.317887] CR0: 80050033 CR2: ffbff000 CR3: 0850e000 CR4: 00150ef0 [ 23.319859] Call Trace: [ 23.320978] ttm_bo_delayed_delete+0x8c/0x94 [ 23.322492] process_one_work+0x21a/0x538 [ 23.323959] worker_thread+0x146/0x398 [ 23.325353] kthread+0xea/0x10c [ 23.326609] ? process_one_work+0x538/0
Re: [Intel-gfx] [BUG 6.3-rc1] Bad lock in ttm_bo_delayed_delete()
On Wed, 15 Mar 2023 20:51:49 +0100 Christian König wrote: > Steven please try the attached patch. I applied it, but as it's not always reproducible, I'll have to give it several runs before I give you my "tested-by" tag. -- Steve
enhancing module info to allow grouping of firmwares
Hey moduly/firmware people, We are facing a problem in the future of NVIDIA providing giant firmwares for their devices that are version locked with unstable APIs. Even if they weren't version locked we'd likely have to support multiple major versions over time. Now this becomes a problem because when you generate multiple initramfs and stick them into /boot over time the number of firmwares MODULE_FIRMWARE will match will increase and dracut will shove them all into the initramfs. I think one way to mitigate that would be to provide some sort of grouping of module firmwares that are acceptable, and having dracut only pick one from the list to provide in the initramfs (hopefully the latest one). That way the driver can provide a list of MODULE_FIRMWARE lines and userspace knows they are a group. I've just little idea how we could expose this via current module info, happy to try and come up with an enhanced scheme maybe with a fallback to just include all of them but was just wanting to get some feedback on whether this was something that anyone has ever considered before now. Thanks, Dave.
Re: enhancing module info to allow grouping of firmwares
On Wed, Mar 15, 2023 at 1:35 PM Dave Airlie wrote: > > Hey moduly/firmware people, > > We are facing a problem in the future of NVIDIA providing giant > firmwares for their devices that are version locked with unstable > APIs. Even if they weren't version locked we'd likely have to support > multiple major versions over time. > > Now this becomes a problem because when you generate multiple > initramfs and stick them into /boot over time the number of firmwares > MODULE_FIRMWARE will match will increase and dracut will shove them > all into the initramfs. > > I think one way to mitigate that would be to provide some sort of > grouping of module firmwares that are acceptable, and having dracut > only pick one from the list to provide in the initramfs (hopefully the > latest one). That way the driver can provide a list of MODULE_FIRMWARE > lines and userspace knows they are a group. > > I've just little idea how we could expose this via current module > info, happy to try and come up with an enhanced scheme maybe with a > fallback to just include all of them but was just wanting to get some > feedback on whether this was something that anyone has ever considered > before now. We have a kind of similar problem with drm/msm and various other SoC drivers that need fw which is signed with a device or vendor specific tree. We've partially solved that with using firmware-name from devicetree to specify the correct device specific fw needs to be loaded (ie. something like "qcom/LENOVO/81JL/qcdxkmsuc850.mbn") but I've no idea how dracut should figure out what fw files should end up in an initrd BR, -R
Re: enhancing module info to allow grouping of firmwares
On Wed, Mar 15, 2023 at 4:35 PM Dave Airlie wrote: > > Hey moduly/firmware people, > > We are facing a problem in the future of NVIDIA providing giant > firmwares for their devices that are version locked with unstable > APIs. Even if they weren't version locked we'd likely have to support > multiple major versions over time. > > Now this becomes a problem because when you generate multiple > initramfs and stick them into /boot over time the number of firmwares > MODULE_FIRMWARE will match will increase and dracut will shove them > all into the initramfs. > > I think one way to mitigate that would be to provide some sort of > grouping of module firmwares that are acceptable, and having dracut > only pick one from the list to provide in the initramfs (hopefully the > latest one). That way the driver can provide a list of MODULE_FIRMWARE > lines and userspace knows they are a group. > > I've just little idea how we could expose this via current module > info, happy to try and come up with an enhanced scheme maybe with a > fallback to just include all of them but was just wanting to get some > feedback on whether this was something that anyone has ever considered > before now. What about bundling the compatible FWs together into one big FW package and tag it with some sort of common metadata header that the driver can parse. That would keep all cross FW compatibilities together. Then on the driver side, change the firmware name specified in the kernel to match whatever is required for that kernel version. E.g., one kernel could specify nv_fw_1_0.bin and another could specify nv_fw_2_1.bin, etc. It's pretty ugly and not a great user experience if there is no backwards compat, but it should work as only the compatible FW would be copied to the initrd. Alex > > Thanks, > Dave.
[PATCH v7 0/2] drm: add kms driver for loongson display controller
Loongson display controller IP has been integrated in both Loongson North Bridge chipset(ls7a1000 and ls7a2000) and Loongson SoCs(ls2k1000 and ls2k2000 etc), it even has been included in Loongson BMC products. This display controller is a PCI device, it has two display pipe. For the DC in LS7A1000 and LS2K1000 each way has a DVO output interface which provide RGB888 signals, vertical & horizontal synchronisations, and the pixel clock. Each CRTC is able to support 1920x1080@60Hz, the maximum resolution is 2048x2048 according to the hardware spec. For the DC in LS7A2000, each display pipe is equipped with a built-in HDMI encoder which is compliant with HDMI 1.4 specification, thus it support 3840x2160@30Hz. The first display pipe is also equipped with a transparent vga encoder which is parallel with the HDMI encoder. The DC in LS7A2000 is more complete, besides above feature, it has two hardware cursors, two hardware vblank counter and two scanout position recorders. v1 -> v2: 1) Use hpd status reg when polling for ls7a2000 2) Fix all warnings emerged when compile with W=1 v2 -> v3: 1) Add COMPILE_TEST in Kconfig and make the driver off by default 2) Alphabetical sorting headers (Thomas) 3) Untangle register access functions as much as possible (Thomas) 4) Switch to TTM based memory manager and prefer cached mapping for Loongson SoC (Thomas) 5) Add chip id detection method, now all models are distinguishable. 6) Revise builtin HDMI phy driver, nearly all main stream mode below 4K@30Hz is tested, this driver supported these mode very well including clone display mode and extend display mode. v3 -> v4: 1) Quickly fix a small mistake. v4 -> v5: 1) Drop potential support for Loongson 2K series SoC temporary, this part should be resend with the DT binding patch in the future. 2) Add per display pipe debugfs support to the builtin HDMI encoder. 3) Rewrite atomic_update() for hardware cursors plane(Thomas) 4) Rewrite encoder and connector initialization part, untangle it according to the chip(Thomas). v5 -> v6: 1) Remove stray code which didn't get used, say lsdc_of_get_reserved_ram 2) Fix all typos I could found, make sentences and code more readable 3) Untange lsdc_hdmi*_connector_detect() function according to the pipe 4) After a serious consideration, we rename this driver as loongson. Because we also have drivers toward the LoongGPU IP in LS7A2000 and LS2K2000. Besides, there are also drivers about the external encoder, HDMI audio driver and vbios support etc. This patch only provide DC driver part, my teammate Li Yi believe that loongson will be more suitable for loongson graphics than lsdc in the long run. loongson.ko = LSDC + LoongGPU + encoders driver + vbios/DT ... v6 -> v7: 1) Add prime support, self-sharing is works. sharing buffer with etnaviv is also tested, and its works with limitation. 2) Implement buffer objects tracking with list_head. 3) S3(sleep to RAM) is tested on ls3a5000+ls7a2000 evb and it works. 4) Rewrite lsdc_bo_move, since ttm core stop allocating resources during BO creation. Patch V1 ~ V6 of this series no longer works on Linux 6.3.0-rc2. Thus, we send V7 to revival them. 5) Remove depends on LOONGARCH || MIPS || COMPILE_TEST and other improvements to help review on x86 machine and add a cover letter to help patchwork tracking my patch. V1 ~ V4 can be found at link [1], V5 at link [2], V6 at link [3] [1] https://patchwork.freedesktop.org/series/113566/ [2] https://patchwork.freedesktop.org/series/114386/ [3] https://patchwork.freedesktop.org/patch/524532/ suijingfeng (2): MAINTAINERS: add maintainers for DRM LOONGSON driver drm: add kms driver for loongson display controller MAINTAINERS | 8 + drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/loongson/Kconfig| 15 + drivers/gpu/drm/loongson/Makefile | 16 + drivers/gpu/drm/loongson/lsdc_crtc.c| 384 drivers/gpu/drm/loongson/lsdc_debugfs.c | 261 +++ drivers/gpu/drm/loongson/lsdc_drv.c | 502 + drivers/gpu/drm/loongson/lsdc_drv.h | 319 ++ drivers/gpu/drm/loongson/lsdc_gem.c | 291 drivers/gpu/drm/loongson/lsdc_gem.h | 26 ++ drivers/gpu/drm/loongson/lsdc_i2c.c | 172 drivers/gpu/drm/loongson/lsdc_irq.c | 88 drivers/gpu/drm/loongson/lsdc_irq.h | 11 + drivers/gpu/drm/loongson/lsdc_output.c | 564 drivers/gpu/drm/loongson/lsdc_output.h | 14 + drivers/gpu/drm/loongson/lsdc_plane.c | 409 + drivers/gpu/drm/loongson/lsdc_pll.c | 338 ++ drivers/gpu/drm/loongson/lsdc_pll.h | 76 drivers/gpu/drm/loongson/lsdc_probe.c | 85 drivers/gpu/drm/loongson/lsdc_probe.h | 11 + drivers/gp
[PATCH v7 1/2] MAINTAINERS: add maintainers for DRM LOONGSON driver
From: suijingfeng This patch add myself as maintainer to fix following warning when run ./scripts/checkpatch.pl WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? Signed-off-by: suijingfeng Signed-off-by: Sui Jingfeng <15330273...@189.cn> --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index abf52201a630..dfacd58c2134 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6909,6 +6909,14 @@ T: git git://anongit.freedesktop.org/drm/drm-misc F: drivers/gpu/drm/lima/ F: include/uapi/drm/lima_drm.h +DRM DRIVERS FOR LOONGSON +M: suijingfeng +L: dri-devel@lists.freedesktop.org +S: Supported +W: https://www.loongson.cn/ +T: git git://anongit.freedesktop.org/drm/drm-misc +F: drivers/gpu/drm/loongson/ + DRM DRIVERS FOR MEDIATEK M: Chun-Kuang Hu M: Philipp Zabel -- 2.25.1
[PATCH v7 2/2] drm: add kms driver for loongson display controller
From: suijingfeng Loongson display controller IP has been integrated in both Loongson North Bridge chipset(ls7a1000 and ls7a2000) and Loongson SoCs(ls2k1000 and ls2k2000 etc), it even has been included in Loongson BMC products. This display controller is a PCI device, it has two display pipe. For the DC in LS7A1000 and LS2K1000 each way has a DVO output interface which provide RGB888 signals, vertical & horizontal synchronisations, and the pixel clock. Each CRTC is able to support 1920x1080@60Hz, the maximum resolution is 2048x2048 according to the hardware spec. For the DC in LS7A2000, each display pipe is equipped with a built-in HDMI encoder which is compliant with HDMI 1.4 specification, thus it support 3840x2160@30Hz. The first display pipe is also equipped with a transparent vga encoder which is parallel with the HDMI encoder. The DC in LS7A2000 is more complete, besides above feature, it has two hardware cursors, two hardware vblank counter and two scanout position recorders. v1 -> v2: 1) Use hpd status reg when polling for ls7a2000 2) Fix all warnings emerged when compile with W=1 v2 -> v3: 1) Add COMPILE_TEST in Kconfig and make the driver off by default 2) Alphabetical sorting headers (Thomas) 3) Untangle register access functions as much as possible (Thomas) 4) Switch to TTM based memory manager and prefer cached mapping for Loongson SoC (Thomas) 5) Add chip id detection method, now all models are distinguishable. 6) Revise builtin HDMI phy driver, nearly all main stream mode below 4K@30Hz is tested, this driver supported these mode very well including clone display mode and extend display mode. v3 -> v4: 1) Quickly fix a small mistake. v4 -> v5: 1) Drop potential support for Loongson 2K series SoC temporary, this part should be resend with the DT binding patch in the future. 2) Add per display pipe debugfs support to the builtin HDMI encoder. 3) Rewrite atomic_update() for hardware cursors plane(Thomas) 4) Rewrite encoder and connector initialization part, untangle it according to the chip(Thomas). v5 -> v6: 1) Remove stray code which didn't get used, say lsdc_of_get_reserved_ram 2) Fix all typos I could found, make sentences and code more readable 3) Untange lsdc_hdmi*_connector_detect() function according to the pipe 4) After a serious consideration, we rename this driver as loongson. Because we also have drivers toward the LoongGPU IP in LS7A2000 and LS2K2000. Besides, there are also drivers about the external encoder, HDMI audio driver and vbios support etc. This patch only provide DC driver part, my teammate Li Yi believe that loongson will be more suitable for loongson graphics than lsdc in the long run. loongson.ko = LSDC + LoongGPU + encoders driver + vbios/DT ... v6 -> v7: 1) Add prime support, self-sharing is works. sharing buffer with etnaviv is also tested, and its works with limitation. 2) Implement buffer objects tracking with list_head. 3) S3(sleep to RAM) is tested on ls3a5000+ls7a2000 evb and it works. 4) Rewrite lsdc_bo_move, since ttm core stop allocating resources during BO creation. Patch V1 ~ V6 of this series no longer works on latest kernel. Thus, we send V7. Signed-off-by: Li Yi Signed-off-by: suijingfeng Signed-off-by: Sui Jingfeng <15330273...@189.cn> --- drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/loongson/Kconfig| 15 + drivers/gpu/drm/loongson/Makefile | 16 + drivers/gpu/drm/loongson/lsdc_crtc.c| 384 drivers/gpu/drm/loongson/lsdc_debugfs.c | 261 +++ drivers/gpu/drm/loongson/lsdc_drv.c | 502 + drivers/gpu/drm/loongson/lsdc_drv.h | 319 ++ drivers/gpu/drm/loongson/lsdc_gem.c | 291 drivers/gpu/drm/loongson/lsdc_gem.h | 26 ++ drivers/gpu/drm/loongson/lsdc_i2c.c | 172 drivers/gpu/drm/loongson/lsdc_irq.c | 88 drivers/gpu/drm/loongson/lsdc_irq.h | 11 + drivers/gpu/drm/loongson/lsdc_output.c | 564 drivers/gpu/drm/loongson/lsdc_output.h | 14 + drivers/gpu/drm/loongson/lsdc_plane.c | 409 + drivers/gpu/drm/loongson/lsdc_pll.c | 338 ++ drivers/gpu/drm/loongson/lsdc_pll.h | 76 drivers/gpu/drm/loongson/lsdc_probe.c | 85 drivers/gpu/drm/loongson/lsdc_probe.h | 11 + drivers/gpu/drm/loongson/lsdc_regs.h| 370 drivers/gpu/drm/loongson/lsdc_ttm.c | 422 ++ drivers/gpu/drm/loongson/lsdc_ttm.h | 71 +++ 23 files changed, 4448 insertions(+) create mode 100644 drivers/gpu/drm/loongson/Kconfig create mode 100644 drivers/gpu/drm/loongson/Makefile create mode 100644 drivers/gpu/drm/loongson/lsdc_crtc.c create mode 100644 drivers/gpu/drm/loongson/lsdc_debugfs.c create mode 100644 drivers/gpu/drm/loongson/lsdc_dr