Re: [PATCH 1/2] drm/mxsfb: Call drm_crtc_vblank_on/off
On Wed, May 27, 2020 at 11:47:56AM +0200, Daniel Vetter wrote: > mxsfb has vblank support, is atomic, but doesn't call > drm_crtc_vblank_on/off as it should. Not good. > > With my next patch to add the drm_crtc_vblank_reset to helpers this > means not even the very first crtc enabling will vblanks work anymore, > since they'll just stay off forever. > > Since mxsfb doesn't have any vblank waits of its own in the > enable/disable flow, nor an enable/disable_vblank callback we can do > the on/off as the first respectively last operation, and it should all > work. > > Signed-off-by: Daniel Vetter > Cc: Marek Vasut > Cc: Stefan Agner > Cc: Shawn Guo > Cc: Sascha Hauer > Cc: Pengutronix Kernel Team > Cc: Fabio Estevam > Cc: NXP Linux Team > Cc: linux-arm-ker...@lists.infradead.org Ping for some ack/review on this one here, it's holding up the subsystem wide fix in patch 2. Thanks, Daniel > --- > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > index 497cf443a9af..1891cd6deb2f 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > @@ -124,6 +124,7 @@ static void mxsfb_pipe_enable(struct > drm_simple_display_pipe *pipe, > drm_panel_prepare(mxsfb->panel); > mxsfb_crtc_enable(mxsfb); > drm_panel_enable(mxsfb->panel); > + drm_crtc_vblank_on(>crtc); > } > > static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe) > @@ -133,6 +134,7 @@ static void mxsfb_pipe_disable(struct > drm_simple_display_pipe *pipe) > struct drm_crtc *crtc = >crtc; > struct drm_pending_vblank_event *event; > > + drm_crtc_vblank_off(>crtc); > drm_panel_disable(mxsfb->panel); > mxsfb_crtc_disable(mxsfb); > drm_panel_unprepare(mxsfb->panel); > -- > 2.26.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] nouveau: add fbdev dependency
On Thu, 28 May 2020 at 00:36, Arnd Bergmann wrote: > > On Wed, May 27, 2020 at 4:05 PM Ilia Mirkin wrote: > > > > Isn't this already fixed by > > > > https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403 > > Ok, I see that fixes the link error, but I when I created my fix, that did > not seem like the correct solution because it reverts part of the original > patch without reverting the rest of it. Unfortunately there was no > changelog text in the first patch to explain why this is safe. No it doesn't, I think you missed the pci in API name. The initial behaviour doesn't use the pci version of the API, the replacement did, and the fix used the drm wrapper around the pci one. So this patch isn't necessary now that I've fixed it the other way, Thanks, Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1 4/4] gpu: host1x: debug: Dump push buffer state
Hi Dmitry, I love your patch! Perhaps something to improve: [auto build test WARNING on tegra-drm/drm/tegra/for-next] [also build test WARNING on v5.7-rc7 next-20200526] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Dmitry-Osipenko/Minor-improvements-for-Host1x-driver/20200525-152833 base: git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next config: arm64-allyesconfig (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 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 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot All warnings (new ones prefixed by >>, old ones prefixed by <<): In file included from drivers/gpu/host1x/hw/host1x01.c:17: drivers/gpu/host1x/hw/debug_hw.c: In function 'show_channel_gathers': >> drivers/gpu/host1x/hw/debug_hw.c:198:44: warning: format '%x' expects >> argument of type 'unsigned int', but argument 3 has type 'dma_addr_t' {aka >> 'long long unsigned int'} [-Wformat=] 198 | host1x_debug_output(o, "Push Buffer at %08x, %d wordsn", | ~~~^ || |unsigned int | %08llx 199 |pb->dma, pb->size / 4); |~~~ | | | dma_addr_t {aka long long unsigned int} vim +198 drivers/gpu/host1x/hw/debug_hw.c 192 193 static void show_channel_gathers(struct output *o, struct host1x_cdma *cdma) 194 { 195 struct push_buffer *pb = >push_buffer; 196 struct host1x_job *job; 197 > 198 host1x_debug_output(o, "Push Buffer at %08x, %d words\n", 199 pb->dma, pb->size / 4); 200 201 show_gather(o, pb->dma, pb->size / 4, cdma, pb->dma, pb->mapped); 202 203 list_for_each_entry(job, >sync_queue, list) { 204 unsigned int i; 205 206 host1x_debug_output(o, "\n%p: JOB, syncpt_id=%d, syncpt_val=%d, first_get=%08x, timeout=%d num_slots=%d, num_handles=%d\n", 207 job, job->syncpt_id, job->syncpt_end, 208 job->first_get, job->timeout, 209 job->num_slots, job->num_unpins); 210 211 for (i = 0; i < job->num_gathers; i++) { 212 struct host1x_job_gather *g = >gathers[i]; 213 u32 *mapped; 214 215 if (job->gather_copy_mapped) 216 mapped = (u32 *)job->gather_copy_mapped; 217 else 218 mapped = host1x_bo_mmap(g->bo); 219 220 if (!mapped) { 221 host1x_debug_output(o, "[could not mmap]\n"); 222 continue; 223 } 224 225 host1x_debug_output(o, "GATHER at %pad+%#x, %d words\n", 226 >base, g->offset, g->words); 227 228 show_gather(o, g->base + g->offset, g->words, cdma, 229 g->base, mapped); 230 231 if (!job->gather_copy_mapped) 232 host1x_bo_munmap(g->bo, mapped); 233 } 234 } 235 } 236 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/4] dt-bindings: display: bridge: thc63lvd1024: Document dual-output mode
On Thu, 14 May 2020 02:21:27 +0300, Laurent Pinchart wrote: > The DT binding support both dual-input and dual-output mode, but only > dual-input is documented. Document dual-output mode. > > Suggested-by: Jacopo Mondi > Signed-off-by: Laurent Pinchart > --- > .../display/bridge/thine,thc63lvd1024.yaml | 16 +++- > 1 file changed, 11 insertions(+), 5 deletions(-) > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/5] dt-bindings: vendor-prefixes: Add Xingbangda
On Wed, 13 May 2020 23:24:47 +0200, Ondrej Jirman wrote: > From: Icenowy Zheng > > Shenzhen Xingbangda Display Technology Co., Ltd is a company which > produces LCD modules. It supplies the LCD panels for the PinePhone. > > Add the vendor prefix of it. > > Signed-off-by: Icenowy Zheng > Signed-off-by: Ondrej Jirman > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/5] dt-bindings: panel: Add binding for Xingbangda XBD599 panel
On Wed, 13 May 2020 23:24:48 +0200, Ondrej Jirman wrote: > From: Icenowy Zheng > > Xingbangda XBD599 is a 5.99" 720x1440 MIPI-DSI LCD panel. It is based on > Sitronix ST7703 LCD controller. > > Add its device tree binding. > > Signed-off-by: Icenowy Zheng > Signed-off-by: Ondrej Jirman > --- > .../display/panel/sitronix,st7703.yaml| 63 +++ > 1 file changed, 63 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/sitronix,st7703.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/bridge: Introduce LT9611 DSI to HDMI bridge
Hi Vinod, Thank you for the patch. On Wed, May 13, 2020 at 03:35:33PM +0530, Vinod Koul wrote: > Lontium Lt9611 is a DSI to HDMI bridge which supports two DSI ports and > I2S port as an input and HDMI port as output > > Co-developed-by: Bjorn Andersson > Signed-off-by: Bjorn Andersson > Signed-off-by: Vinod Koul > --- > drivers/gpu/drm/bridge/Kconfig | 13 + > drivers/gpu/drm/bridge/Makefile |1 + > drivers/gpu/drm/bridge/lt9611.c | 1113 +++ > 3 files changed, 1127 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/lt9611.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index aaed2347ace9..5cac15ce2027 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -38,6 +38,19 @@ config DRM_DISPLAY_CONNECTOR > on ARM-based platforms. Saying Y here when this driver is not needed > will not cause any issue. > > +config DRM_LONTIUM_LT9611 > + tristate "Lontium LT9611 DSI/HDMI bridge" > + select SND_SOC_HDMI_CODEC if SND_SOC > + depends on OF > + select DRM_PANEL_BRIDGE > + select DRM_KMS_HELPER > + select REGMAP_I2C > + help > + Driver for Lontium LT9611 DSI to HDMI bridge > + chip driver that converts dual DSI and I2S to > + HDMI signals > + Please say Y if you have such hardware. > + > config DRM_LVDS_CODEC > tristate "Transparent LVDS encoders and decoders support" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 6fb062b5b0f0..d2a696e8ca5d 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > +obj-$(CONFIG_DRM_LONTIUM_LT9611) += lt9611.o > obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o > obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += > megachips-stdp-ge-b850v3-fw.o > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o > diff --git a/drivers/gpu/drm/bridge/lt9611.c b/drivers/gpu/drm/bridge/lt9611.c > new file mode 100644 > index ..e6e7ce43980d > --- /dev/null > +++ b/drivers/gpu/drm/bridge/lt9611.c > @@ -0,0 +1,1113 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + * Copyright (c) 2019-2020. Linaro Limited. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define EDID_SEG_SIZE 256 > + > +#define LT9611_4LANES0 > + > +struct lt9611 { > + struct device *dev; > + struct drm_bridge bridge; > + struct drm_connector connector; > + > + struct regmap *regmap; > + > + struct device_node *dsi0_node; > + struct device_node *dsi1_node; > + struct mipi_dsi_device *dsi0; > + struct mipi_dsi_device *dsi1; > + > + bool ac_mode; > + > + struct gpio_desc *reset_gpio; > + struct gpio_desc *enable_gpio; > + > + bool power_on; > + bool sleep; > + > + struct regulator_bulk_data supplies[2]; > + > + struct i2c_client *client; > + > + enum drm_connector_status status; > + > + u8 edid_buf[EDID_SEG_SIZE]; > + u32 vic; > +}; > + > +#define LT9611_PAGE_CONTROL 0xff > + > +static const struct regmap_range_cfg lt9611_ranges[] = { > + { > + .name = "register_range", > + .range_min = 0, > + .range_max = 0x85ff, > + .selector_reg = LT9611_PAGE_CONTROL, > + .selector_mask = 0xff, > + .selector_shift = 0, > + .window_start = 0, > + .window_len = 0x100, > + }, > +}; > + > +static const struct regmap_config lt9611_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = 0x, > + .ranges = lt9611_ranges, > + .num_ranges = ARRAY_SIZE(lt9611_ranges), > +}; > + > +struct lt9611_mode { > + u16 hdisplay; > + u16 vdisplay; > + u8 fps; > + u8 lanes; > + u8 intfs; > +}; > + > +static struct lt9611_mode lt9611_modes[] = { > + { 3840, 2160, 30, 4, 2 }, /* 3840x2160 24bit 30Hz 4Lane 2ports */ > + { 1920, 1080, 60, 4, 1 }, /* 1080P 24bit 60Hz 4lane 1port */ > + { 1920, 1080, 30, 3, 1 }, /* 1080P 24bit 30Hz 3lane 1port */ > + { 1920, 1080, 24, 3, 1 }, > + { 720, 480, 60, 4, 1 }, > + { 720, 576, 50, 2, 1 }, > + { 640, 480, 60, 2, 1 }, > +}; > + > +static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge) > +{ > + return container_of(bridge, struct lt9611, bridge); > +} > + > +static struct lt9611 *connector_to_lt9611(struct drm_connector *connector) > +{ > + return container_of(connector, struct lt9611, connector); > +} > + > +static int lt9611_mipi_input_analog(struct lt9611 *lt9611) > +{ > + struct reg_sequence reg_cfg[] = { > +
Re: [PATCH 2/3] dt-bindings: display: bridge: Add documentation for LT9611
Hi Vinod, Thank you for the patch. On Wed, May 13, 2020 at 03:35:32PM +0530, Vinod Koul wrote: > Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports > and I2S port as input and one HDMI port as output > > Signed-off-by: Vinod Koul > --- > .../display/bridge/lontium,lt9611.yaml| 178 ++ > 1 file changed, 178 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml > b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml > new file mode 100644 > index ..77ee8cc35cd8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml > @@ -0,0 +1,178 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Lontium LT9611 2 Port MIPI to HDMI Bridge > + > +maintainers: > + - Vinod Koul > + > +description: | > + The LT9611 is a bridge device which converts DSI to HDMI > + > +properties: > + compatible: > +enum: > + - lontium,lt9611 > + > + reg: > +maxItems: 1 > +description: base I2C address of the device. > + > + "#sound-dai-cells": > +const: 1 > + > + interrupts: > +maxItems: 1 > +description: interrupt line for the chip I think you could drop the descriptions for the reg and interrupt properties, they don't add much. > + > + reset-gpios: > +maxItems: 1 > +description: GPIO connected to active high RESET pin. > + > + vdd-supply: > +description: Regulator for 1.8V MIPI phy power. > + > + vcc-supply: > +description: Regulator for 3.3V IO power. > + > + ports: > +type: object > + > +properties: > + "#address-cells": > +const: 1 > + > + "#size-cells": > +const: 0 > + > + port@0: > +type: object > +additionalProperties: false > + > +description: | > + HDMI port for HDMI output The usual practice is to have the input ports first, followed by the output ports. Is there a reason not to follow that rule ? > + > +properties: > + reg: > +const: 0 > + > +patternProperties: > + endpoint: If you want to use patternProperties, this should be "^endpoint@[0-9]+$": (including the quotes). Same below. > +type: object > +additionalProperties: false > + > +properties: > + remote-endpoint: true How about remote-endpoint: $ref: /schemas/types.yaml#/definitions/phandle and the same below ? You also need a reg property if multiple endpoints are present. > + > +required: > + - reg > + > + port@1: > +type: object > +additionalProperties: false > + > +description: | > + MIPI port-1 for MIPI input > + > +properties: > + reg: > +const: 1 > + > +patternProperties: > + endpoint: > +type: object > +additionalProperties: false > + > +properties: > + remote-endpoint: true > + > +required: > + - reg > + > + port@2: > +type: object > +additionalProperties: false > + > +description: | > + MIPI port-2 for MIPI input A description of how the two MIPI inputs differ would be useful. In particular, are both mandatory, or is it valid to connect only one of the two ? If using a single input is supported, can it be either, or does it have to be the first one ? When using both inputs, what should be connected to them ? > + > +properties: > + reg: > +const: 2 > + > +patternProperties: > + endpoint: > +type: object > +additionalProperties: false > + > +properties: > + remote-endpoint: true > + > +required: > + - reg > + > +required: > + - "#address-cells" > + - "#size-cells" > + - port@0 > + - port@1 > + > +required: > + - compatible > + - reg > + - interrupts > + - vdd-supply > + - vcc-supply > + - ports > + > +additionalProperties: false > + > +examples: > + - | > +#include > +#include > + > +i2c10 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + lt9611_codec: hdmi-bridge@3b { Please drop unused labels. > +compatible = "lontium,lt9611"; > +reg = <0x3b>; > + > +reset-gpios = < 128 GPIO_ACTIVE_HIGH>; > +interrupts-extended = < 84 IRQ_TYPE_EDGE_FALLING>; > + > +vdd-supply = <_1v8>; > +vcc-supply = <_3v3>; > + > +ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > +reg = <0>; > +
Re: [PATCH 2/3] dt-bindings: display: bridge: Add documentation for LT9611
On Wed, 13 May 2020 15:35:32 +0530, Vinod Koul wrote: > Lontium LT9611 is a DSI to HDMI bridge which supports 2 DSI ports > and I2S port as input and one HDMI port as output > > Signed-off-by: Vinod Koul > --- > .../display/bridge/lontium,lt9611.yaml| 178 ++ > 1 file changed, 178 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml > Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] dt-bindings: vendor-prefixes: Add Lontium vendor prefix
On Wed, 13 May 2020 15:35:31 +0530, Vinod Koul wrote: > Add prefix for Lontium Semiconductor Corporation > > Signed-off-by: Vinod Koul > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp
Hi, Jitao: Jitao Shi 於 2020年5月22日 週五 下午6:12寫道: > > If panel has too small hfp or hbp, horizontal_frontporch_byte or > horizontal_backporch_byte may become very small value or negative > value. This patch adjusts their values so that they are greater > than minimum value and keep total of them unchanged. > > Signed-off-by: Jitao Shi > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 0ede69830a9d..aebaafd90ceb 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -148,6 +148,9 @@ > (type == MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM) || \ > (type == MIPI_DSI_DCS_READ)) > > +#define MIN_HFP_BYTE 0x02 > +#define MIN_HBP_BYTE 0x02 > + > struct mtk_phy_timing { > u32 lpx; > u32 da_hs_prepare; > @@ -450,6 +453,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) > u32 horizontal_sync_active_byte; > u32 horizontal_backporch_byte; > u32 horizontal_frontporch_byte; > + s32 signed_hfp_byte, signed_hbp_byte; > u32 dsi_tmp_buf_bpp, data_phy_cycles; > struct mtk_phy_timing *timing = >phy_timing; > > @@ -519,6 +523,20 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi > *dsi) > } > } > > + signed_hfp_byte = (s32)horizontal_frontporch_byte; > + signed_hbp_byte = (s32)horizontal_backporch_byte; > + > + if (signed_hfp_byte + signed_hbp_byte < MIN_HFP_BYTE + MIN_HBP_BYTE) { > + DRM_WARN("Calculated hfp_byte and hbp_byte are too small, " > +"panel may not work properly.\n"); > + } else if (signed_hfp_byte < MIN_HFP_BYTE) { > + horizontal_frontporch_byte = MIN_HFP_BYTE; > + horizontal_backporch_byte -= MIN_HFP_BYTE - signed_hfp_byte; > + } else if (signed_hbp_byte < MIN_HBP_BYTE) { > + horizontal_frontporch_byte -= MIN_HBP_BYTE - signed_hbp_byte; > + horizontal_backporch_byte = MIN_HBP_BYTE; > + } > + I think horizontal_frontporch_byte would never be negtive, and horizontal_backporch_byte would be negtive when if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) horizontal_backporch_byte = (vm->hback_porch * dsi_tmp_buf_bpp - 10); else horizontal_backporch_byte = ((vm->hback_porch + vm->hsync_len) * dsi_tmp_buf_bpp - 10); If this time it's negtive, the calculation of horizontal_frontporch_byte is so strange. I think processing negtive value should before here. Regards, Chun-Kuang. > writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC); > writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC); > writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC); > -- > 2.25.1 > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[pull] amdgpu, amdkfd drm-next-5.8
Hi Dave, Daniel, Fixes for 5.8. The following changes since commit c41219fda6e04255c44d37fd2c0d898c1c46abf1: Merge tag 'drm-intel-next-fixes-2020-05-20' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2020-05-21 10:44:33 +1000) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/amd-drm-next-5.8-2020-05-27 for you to fetch changes up to 185082b679b4bd6dfb69764eaa89213b26f6f703: drm/amd/display: Fix potential integer wraparound resulting in a hang (2020-05-27 18:42:10 -0400) amd-drm-next-5.8-2020-05-27: amdgpu: - SRIOV fixes - RAS fixes - VCN 2.5 DPG (Dynamic PowerGating) fixes - FP16 updates for display - CTF cleanups - Display fixes - Fix pcie bw sysfs handling - Enable resizeable BAR support for gmc 10.x - GFXOFF fixes for Raven - PM sysfs handling fixes amdkfd: - Fix a race condition - Warning fixes Aishwarya Ramakrishnan (1): drm/amdkfd: Fix boolreturn.cocci warnings Alan Swanson (1): drm/amdgpu: resize VRAM BAR for CPU access on gfx10 Alex Deucher (6): drm/amdgpu: improve error handling in pcie_bw drm/amdgpu: drop navi pcie bw callback drm/amdgpu: move discovery gfx config fetching drm/amdgpu: move gpu_info parsing after common early init drm/amdgpu: fix pm sysfs node handling (v2) drm/amdgpu: add apu flags (v2) Aric Cyr (1): drm/amd/display: Fix potential integer wraparound resulting in a hang Bhawanpreet Lakha (1): drm/amd/display: Handle GPU reset for DC block Dan Carpenter (1): drm/amdgpu: off by one in amdgpu_device_attr_create_groups() error handling Dmytro Laktyushkin (2): drm/amd/display: fix and simplify pipe split logic drm/amd/display: correct rn NUM_VMID Evan Quan (2): drm/amd/powerplay: unify the prompts on thermal interrupts drm/amdkfd: report the real PCI bus number Felix Kuehling (1): drm/amdgpu: Sync with VM root BO when switching VM to CPU update mode Gustavo A. R. Silva (1): drm/amdgpu/smu10: Replace one-element array and use struct_size() helper Harry Wentland (3): drm/amd/display: Add DC Debug mask to disable features for bringup drm/amd/display: Fix disable_stutter debug option drm/amd/display: Respect PP_STUTTER_MODE but don't override DC_DISABLE_STUTTER Jack Zhang (1): drm/amdgpu fix incorrect sysfs remove behavior for xgmi James Zhu (2): drm/amdgpu/jpeg2.5: Remove JPEG_ENC_MASK from clock ungating drm/amdgpu/vcn2.5: Remove old DPG workaround Jinze Xu (1): drm/amd/display: Set/Reset avmute when disable/enable stream John Clements (1): drm/amdgpu: resolve ras recovery vs smi race condition Kevin Wang (2): drm/amdgpu: cleanup unnecessary virt sriov check in amdgpu attribute drm/amdgpu: fix device attribute node create failed with multi gpu Likun Gao (1): drm/amdgpu: add condition to set MP1 state on gpu reset Mario Kleiner (2): drm/amd/display: Expose support for xBGR ordered fp16 formats. drm/amd/display: Enable fp16 also on DCE-11.0 - DCE-12. (v2) Nicholas Kazlauskas (2): drm/amd/display: Defer cursor lock until after VUPDATE drm/amd/display: Avoid pipe split when plane is too small Nikola Cornij (1): drm/amd/display: Minimize DSC resource re-assignment Philip Yang (1): drm/amdkfd: fix restore worker race condition Rodrigo Siqueira (1): drm/amd/display: Remove dml_common_def file Simon Ser (1): drm/amd/display: drop cursor position check in atomic test Stylon Wang (1): drm/amd/display: Fix incorrectly pruned modes with deep color Vladimir Stempen (1): drm/amd/display: DP training to set properly SCRAMBLING_DISABLE chen gong (1): drm/amd/powerpay: Disable gfxoff when setting manual mode on picasso and raven drivers/gpu/drm/amd/amdgpu/amdgpu.h| 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 19 +- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 8 + drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 212 +- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h | 13 +- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c| 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 11 +- drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 23 +- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 +- drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 14 +- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 5 +- drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c
Re: [RFC PATCH 2/2] drm: xlnx: driver for Xilinx DSI TX Subsystem
Hi Hyun, On Wed, May 27, 2020 at 10:54:35AM -0700, Hyun Kwon wrote: > On Sat, 2020-05-23 at 20:08:13 -0700, Laurent Pinchart wrote: > > On Mon, May 04, 2020 at 11:43:48AM -0700, Hyun Kwon wrote: > >> On Mon, 2020-04-20 at 14:20:56 -0700, Venkateshwar Rao Gannavarapu wrote: > >>> The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video > >>> data from AXI-4 stream interface. > >>> > >>> It supports upto 4 lanes, optional register interface for the DPHY, > >> > >> I don't see the register interface for dphy support. > > > > I think the D-PHY should be supported through a PHY driver, as it seems > > to be shared between different subsystems. > > Right, if the logic is shared across subsystems. I can't tell if that's > the case as the IP comes as a single block. Maybe GVRao can confirm. I believe the CSI2-RX subsystem uses the same D-PHY IP core, but a confirmation would be nice. > >>> multiple RGB color formats, command mode and video mode. > >>> This is a MIPI-DSI host driver and provides DSI bus for panels. > >>> This driver also helps to communicate with its panel using panel > >>> framework. > >>> > >>> Signed-off-by: Venkateshwar Rao Gannavarapu > >>> > >>> --- > >>> drivers/gpu/drm/xlnx/Kconfig| 11 + > >>> drivers/gpu/drm/xlnx/Makefile | 2 + > >>> drivers/gpu/drm/xlnx/xlnx_dsi.c | 755 > >>> > > > > Daniel Vetter has recently expressed his opiion that bridge drivers > > should go to drivers/gpu/drm/bridge/. It would then be > > drivers/gpu/drm/bridge/xlnx/. I don't have a strong opinion myself. > > > >>> 3 files changed, 768 insertions(+) > >>> create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c > >>> > >>> diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig > >>> index aa6cd88..73873cf 100644 > >>> --- a/drivers/gpu/drm/xlnx/Kconfig > >>> +++ b/drivers/gpu/drm/xlnx/Kconfig > >>> @@ -11,3 +11,14 @@ config DRM_ZYNQMP_DPSUB > >>> This is a DRM/KMS driver for ZynqMP DisplayPort controller. Choose > >>> this option if you have a Xilinx ZynqMP SoC with DisplayPort > >>> subsystem. > >>> + > >>> +config DRM_XLNX_DSI > >>> +tristate "Xilinx DRM DSI Subsystem Driver" > >>> +select DRM_MIPI_DSI > >>> +select DRM_PANEL > >>> +select DRM_PANEL_SIMPLE > >>> +help > >>> + This enables support for Xilinx MIPI-DSI. > >> > >> This sentence is not needed with below. Could you please rephrase the > >> whole? > >> > >>> + This is a DRM/KMS driver for Xilinx programmable DSI controller. > >>> + Choose this option if you have a Xilinx MIPI DSI-TX controller > >>> + subsytem. > >> > >> These seem incorrectly indented. > >> > >>> diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile > >>> index 2b844c6..b7ee6ef 100644 > >>> --- a/drivers/gpu/drm/xlnx/Makefile > >>> +++ b/drivers/gpu/drm/xlnx/Makefile > >>> @@ -1,2 +1,4 @@ > >>> zynqmp-dpsub-objs += zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o > >>> obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o > >>> + > >>> +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o > >>> diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c > >>> b/drivers/gpu/drm/xlnx/xlnx_dsi.c > >>> new file mode 100644 > >>> index 000..b8cae59 > >>> --- /dev/null > >>> +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c > >>> @@ -0,0 +1,755 @@ > >>> +// SPDX-License-Identifier: GPL-2.0 > >>> +/* > >>> + * Xilinx FPGA MIPI DSI Tx Controller driver > >>> + * > >>> + * Copyright (C) 2017 - 2019 Xilinx, Inc. > >>> + * > >>> + * Authors: > >>> + * - Saurabh Sengar > >>> + * - Venkateshwar Rao Gannavarapu > >>> > >>> + */ > >>> + > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> + > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> + > >>> +#include > >>> +#include > >>> + > >>> +/* DSI Tx IP registers */ > >>> +#define XDSI_CCR 0x00 > >>> +#define XDSI_CCR_COREENB BIT(0) > >>> +#define XDSI_CCR_SOFTRST BIT(1) > >>> +#define XDSI_CCR_CRREADY BIT(2) > >>> +#define XDSI_CCR_CMDMODE BIT(3) > >>> +#define XDSI_CCR_DFIFORSTBIT(4) > >>> +#define XDSI_CCR_CMDFIFORST BIT(5) > >>> +#define XDSI_PCR 0x04 > > [snip] > > >>> + } > >>> + > >>> + ret = clk_prepare_enable(dsi->video_aclk); > >>> + if (ret) { > >>> + dev_err(dev, "failed to enable video clk %d\n", ret); > >>> + goto err_disable_dphy_clk; > >>> + } > >>> + > >>> + ret = component_add(dev, _dsi_component_ops); > > > > The driver should expose the DSI-TX as a drm_bridge instead of using the > > component framework. You shouldn't register a drm_encoder, and I don't > > think you should register a drm_connector either. Only bridge operations > > should be exposed, and the drm_bridge
[pull] amdgpu drm-fixes-5.7
Hi Dave, Daniel, A couple of small fixes for 5.7 The following changes since commit 7d9ff5eed4146bf026c69e766ff630bc0bd555bb: Merge tag 'amd-drm-fixes-5.7-2020-05-21' of git://people.freedesktop.org/~agd5f/linux into drm-fixes (2020-05-22 10:30:51 +1000) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/amd-drm-fixes-5.7-2020-05-27 for you to fetch changes up to 4e5183200d9b66695c754ef214933402056e7b95: drm/amd/display: Fix potential integer wraparound resulting in a hang (2020-05-27 18:13:14 -0400) amd-drm-fixes-5.7-2020-05-27: amdgpu: - Display atomic test fix - Fix soft hang in display vupdate code Aric Cyr (1): drm/amd/display: Fix potential integer wraparound resulting in a hang Simon Ser (1): drm/amd/display: drop cursor position check in atomic test drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 --- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 2 ++ 2 files changed, 2 insertions(+), 7 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH] drm/msm/dpu: ensure device suspend happens during PM sleep
Hi, On Fri, May 15, 2020 at 9:37 AM Doug Anderson wrote: > > Hi, > > On Fri, May 15, 2020 at 5:06 AM wrote: > > > > On 2020-05-14 21:47, Doug Anderson wrote: > > > Hi, > > > > > > On Fri, May 1, 2020 at 6:31 AM Kalyan Thota > > > wrote: > > >> > > >> "The PM core always increments the runtime usage counter > > >> before calling the ->suspend() callback and decrements it > > >> after calling the ->resume() callback" > > >> > > >> DPU and DSI are managed as runtime devices. When > > >> suspend is triggered, PM core adds a refcount on all the > > >> devices and calls device suspend, since usage count is > > >> already incremented, runtime suspend was not getting called > > >> and it kept the clocks on which resulted in target not > > >> entering into XO shutdown. > > >> > > >> Add changes to force suspend on runtime devices during pm sleep. > > >> > > >> Changes in v1: > > >> - Remove unnecessary checks in the function > > >> _dpu_kms_disable_dpu (Rob Clark). > > >> > > >> Changes in v2: > > >> - Avoid using suspend_late to reset the usagecount > > >>as suspend_late might not be called during suspend > > >>call failures (Doug). > > >> > > >> Changes in v3: > > >> - Use force suspend instead of managing device usage_count > > >>via runtime put and get API's to trigger callbacks (Doug). > > >> > > >> Changes in v4: > > >> - Check the return values of pm_runtime_force_suspend and > > >>pm_runtime_force_resume API's and pass appropriately (Doug). > > >> > > >> Changes in v5: > > > > > > Can you please put the version number properly in your subject? It's > > > really hard to tell one version of your patch from another. > > > > > > > > >> - With v4 patch, test cycle has uncovered issues in device resume. > > >> > > >>On bubs: cmd tx failures were seen as SW is sending panel off > > >>commands when the dsi resources are turned off. > > >> > > >>Upon suspend, DRM driver will issue a NULL composition to the > > >>dpu, followed by turning off all the HW blocks. > > >> > > >>v5 changes will serialize the NULL commit and resource unwinding > > >>by handling them under PM prepare and PM complete phases there by > > >>ensuring that clks are on when panel off commands are being > > >>processed. > > > > > > I'm still most definitely not an expert in how all the DRM pieces all > > > hook up together, but the solution you have in this patch seems wrong > > > to me. As far as I can tell the "prepare" state isn't supposed to be > > > actually doing the suspend work and here that's exactly what you're > > > doing. I think you should find a different solution to ensure > > > ordering is correct. > > > > > > -Doug > > > > > > > Hi, > > Quite honestly I'm probably not the right person to be reviewing this > code. I mostly just noticed one of your early patches and it looked > strange to me. Hopefully someone with actual experience in how all > the DRM components work together can actually review and see if this > makes sense. Maybe Sean would know better? > > That being said, let me at least look at what you're saying... > > > > Prepare and Complete are callbacks defined as part of Sleep and Resume > > sequence > > > > Entering PM SUSPEND the phases are : prepare --> suspend --> > > suspend_late --> suspend_noirq. > > While leaving PM SUSPEND the phases are: resume_noirq --> resume_early > > --> resume --> complete. > > Sure, it's part of the sequence. It's also documented in pm.h as: > > * The principal role of this callback is to prevent new children of > * the device from being registered after it has returned (the driver's > * subsystem and generally the rest of the kernel is supposed to prevent > * new calls to the probe method from being made too once @prepare() has > * succeeded). > > It does not feel like that matches your usage of this call. > > > > The reason to push drm suspend handling to PM prepare phase is that > > parent here will trigger a modeset to turn off the timing and > > subsequently the panel. > > the child devices should not turn of their clocks before parent unwinds > > the composition. Hence they are serialized as per the sequence mentioned > > above. > > So the general model in Linux is that children suspend before their > parents, right? So you're saying that, in this case, the parent needs > to act on the child before the child suspends. Is that correct? > > Rather than hijacking the prepare/complete, I'd be at least slightly > inclined to move the other driver to turn off its clocks in > suspend_late and to turn them back on in resume_early? That seems to > be what was done in "analogix_dp-rockchip.c" to solve a similar > problem. > > > > A similar approach is taken by other driver that use drm framework. In > > this driver, the device registers for prepare and complete callbacks to > > handle drm_suspend and drm_resume. > > https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/exynos/exynos_drm_drv.c#L163 >
Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
On Wed, May 27, 2020 at 08:38:47AM -0700, Rob Clark wrote: > On Wed, May 27, 2020 at 1:47 AM Sharat Masetty > wrote: > > > > + more folks > > > > On 5/18/2020 9:55 PM, Rob Clark wrote: > > > On Mon, May 18, 2020 at 7:23 AM Jordan Crouse > > > wrote: > > >> On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote: > > >>> This patches replaces the previously used static DDR vote and uses > > >>> dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling > > >>> GPU frequency. > > >>> > > >>> Signed-off-by: Sharat Masetty > > >>> --- > > >>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +- > > >>> 1 file changed, 1 insertion(+), 5 deletions(-) > > >>> > > >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > >>> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > >>> index 2d8124b..79433d3 100644 > > >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > > >>> @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct > > >>> dev_pm_opp *opp) > > >>> > > >>>gmu->freq = gmu->gpu_freqs[perf_index]; > > >>> > > >>> - /* > > >>> - * Eventually we will want to scale the path vote with the > > >>> frequency but > > >>> - * for now leave it at max so that the performance is nominal. > > >>> - */ > > >>> - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > > >>> + dev_pm_opp_set_bw(>pdev->dev, opp); > > >>> } > > >> This adds an implicit requirement that all targets need bandwidth > > >> settings > > >> defined in the OPP or they won't get a bus vote at all. I would prefer > > >> that > > >> there be an default escape valve but if not you'll need to add > > >> bandwidth values for the sdm845 OPP that target doesn't regress. > > >> > > > it looks like we could maybe do something like: > > > > > >ret = dev_pm_opp_set_bw(...); > > >if (ret) { > > >dev_warn_once(dev, "no bandwidth settings"); > > >icc_set_bw(...); > > >} > > > > > > ? > > > > > > BR, > > > -R > > > > There is a bit of an issue here - Looks like its not possible to two icc > > handles to the same path. Its causing double enumeration of the paths > > in the icc core and messing up path votes. With [1] Since opp/core > > already gets a handle to the icc path as part of table add, drm/msm > > could do either > > > > a) Conditionally enumerate gpu->icc_path handle only when pm/opp core > > has not got the icc path handle. I could use something like [2] to > > determine if should initialize gpu->icc_path* > > > > b) Add peak-opp-configs in 845 dt and mandate all future versions to use > > this bindings. With this, I can remove gpu->icc_path from msm/drm > > completely and only rely on opp/core for bw voting. > > The main thing is that we want to make sure newer dtb always works on > an older kernel without regression.. but, hmm.. I guess the > interconnects/interconnects-names properties haven't landed yet in > sdm845.dtsi? Maybe that lets us go with the simpler approach (b). > Looks like we haven't wired up interconnect for 8916 or 8996 either, > so probably we can just mandate this for all of them? > > If we have landed the interconnect dts hookup for gpu somewhere that > I'm overlooking, I guess we would have to go with (a) and keep the > existing interconnects/interconnects-names properties. The main problem is that (on sdm845 at least) the path comes up with a very slow default so even if we don't do scaling we had to set _something_. Perhaps if we solved that problem somewhere else (inerconnect, rpmh?) then we wouldn't need to worry about it in the leaf driver unless the full opp tables are described. Jordan > BR, > -R > > > [1] - https://lore.kernel.org/patchwork/cover/1240687/ > > > > [2] - https://patchwork.kernel.org/patch/11527573/ > > > > Let me know your thoughts > > > > Sharat > > > > > > > >> Jordan > > >> > > >>> unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) > > >>> -- > > >>> 2.7.4 > > >>> > > >> -- > > >> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > > >> a Linux Foundation Collaborative Project > > >> ___ > > >> Freedreno mailing list > > >> freedr...@lists.freedesktop.org > > >> https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
On 2020-05-27 23:01, Saravana Kannan wrote: On Wed, May 27, 2020 at 8:38 AM Rob Clark wrote: On Wed, May 27, 2020 at 1:47 AM Sharat Masetty wrote: > > + more folks > > On 5/18/2020 9:55 PM, Rob Clark wrote: > > On Mon, May 18, 2020 at 7:23 AM Jordan Crouse wrote: > >> On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote: > >>> This patches replaces the previously used static DDR vote and uses > >>> dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling > >>> GPU frequency. > >>> > >>> Signed-off-by: Sharat Masetty > >>> --- > >>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +- > >>> 1 file changed, 1 insertion(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>> index 2d8124b..79433d3 100644 > >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>> @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) > >>> > >>>gmu->freq = gmu->gpu_freqs[perf_index]; > >>> > >>> - /* > >>> - * Eventually we will want to scale the path vote with the frequency but > >>> - * for now leave it at max so that the performance is nominal. > >>> - */ > >>> - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > >>> + dev_pm_opp_set_bw(>pdev->dev, opp); > >>> } > >> This adds an implicit requirement that all targets need bandwidth settings > >> defined in the OPP or they won't get a bus vote at all. I would prefer that > >> there be an default escape valve but if not you'll need to add > >> bandwidth values for the sdm845 OPP that target doesn't regress. > >> > > it looks like we could maybe do something like: > > > >ret = dev_pm_opp_set_bw(...); > >if (ret) { > >dev_warn_once(dev, "no bandwidth settings"); > >icc_set_bw(...); > >} > > > > ? > > > > BR, > > -R > > There is a bit of an issue here - Looks like its not possible to two icc > handles to the same path. Its causing double enumeration of the paths > in the icc core and messing up path votes. With [1] Since opp/core > already gets a handle to the icc path as part of table add, drm/msm > could do either Are you sure this is the real issue? I'd be surprised if this is a real limitation. And if it is, it either needs to be fixed in the ICC framework or OPP shouldn't be getting path handles by default (and not really, this is already handled well in the icc framework. In this case the max peak vote would be considered among the two paths. maybe let the driver set the handles before using OPP APIs to change BW). I'd lean towards the former. https://patchwork.kernel.org/patch/11573827/ Yes the core shouldn't get paths by default unless the bw values are specified in the opps. > a) Conditionally enumerate gpu->icc_path handle only when pm/opp core > has not got the icc path handle. I could use something like [2] to > determine if should initialize gpu->icc_path* This seems like a bandaid. Let's fix it correctly in ICC framework or OPP framework. > b) Add peak-opp-configs in 845 dt and mandate all future versions to use I can't understand ^^ proposal as well. We would ideally want to add scaling support for SDM845 as well while we are at it. > this bindings. With this, I can remove gpu->icc_path from msm/drm > completely and only rely on opp/core for bw voting. I don't know what you mean by "peak-opp-configs" but I guess you are referring to some kind of DT flag to say if you should vote for BW directly or use the OPP framework? If so, I'm pretty sure that won't fly. That's an OS implementation specific flag. -Saravana -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: device hot-unplug for userspace
On Wed, May 27, 2020 at 9:44 PM Christian König wrote: > > Am 27.05.20 um 17:23 schrieb Andrey Grodzovsky: > > > > On 5/27/20 10:39 AM, Daniel Vetter wrote: > >> On Wed, May 27, 2020 at 3:51 PM Andrey Grodzovsky > >> wrote: > >>> > >>> On 5/27/20 2:44 AM, Pekka Paalanen wrote: > On Tue, 26 May 2020 10:30:20 -0400 > Andrey Grodzovsky wrote: > > > On 5/19/20 6:06 AM, Pekka Paalanen wrote: > >> From: Pekka Paalanen > >> > >> Set up the expectations on how hot-unplugging a DRM device should > >> look like to > >> userspace. > >> > >> Written by Daniel Vetter's request and largely based on his > >> comments in IRC and > >> from > >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-May%2F265484.htmldata=02%7C01%7CAndrey.Grodzovsky%40amd.com%7C3c671803b2ba41b2ceac08d8024bcc9a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637261871869742519sdata=ZnhylRubOM0%2BjoreSSYMqVDzZuUdybEsoyBVcTKgxWE%3Dreserved=0 > >> . > >> > >> Signed-off-by: Pekka Paalanen > >> Cc: Daniel Vetter > >> Cc: Andrey Grodzovsky > >> Cc: Dave Airlie > >> Cc: Sean Paul > >> > >> --- > >> > >> Disclaimer: I am a userspace developer writing for other > >> userspace developers. > >> I took some liberties in defining what should happen without > >> knowing what is > >> actually possible or what existing drivers already implement. > >> --- > >> Documentation/gpu/drm-uapi.rst | 75 > >> ++ > >> 1 file changed, 75 insertions(+) > >> > >> diff --git a/Documentation/gpu/drm-uapi.rst > >> b/Documentation/gpu/drm-uapi.rst > >> index 56fec6ed1ad8..80db4abd2cbd 100644 > >> --- a/Documentation/gpu/drm-uapi.rst > >> +++ b/Documentation/gpu/drm-uapi.rst > >> @@ -1,3 +1,5 @@ > >> +.. Copyright 2020 DisplayLink (UK) Ltd. > >> + > >> === > >> Userland interfaces > >> === > >> @@ -162,6 +164,79 @@ other hand, a driver requires shared state > >> between clients which is > >> visible to user-space and accessible beyond open-file > >> boundaries, they > >> cannot support render nodes. > >> > >> +Device Hot-Unplug > >> += > >> + > >> +.. note:: > >> + The following is the plan. Implementation is not there yet > >> + (2020 May 13). > >> + > >> +Graphics devices (display and/or render) may be connected via > >> USB (e.g. > >> +display adapters or docking stations) or Thunderbolt (e.g. > >> eGPU). An end > >> +user is able to hot-unplug this kind of devices while they are > >> being > >> +used, and expects that the very least the machine does not > >> crash. Any > >> +damage from hot-unplugging a DRM device needs to be limited as > >> much as > >> +possible and userspace must be given the chance to handle it if > >> it wants > >> +to. Ideally, unplugging a DRM device still lets a desktop to > >> continue > >> +running, but that is going to need explicit support throughout > >> the whole > >> +graphics stack: from kernel and userspace drivers, through display > >> +servers, via window system protocols, and in applications and > >> libraries. > >> + > >> +Other scenarios that should lead to the same are: unrecoverable GPU > >> +crash, PCI device disappearing off the bus, or forced unbind of > >> a driver > >> +from the physical device. > >> + > >> +In other words, from userspace perspective everything needs to > >> keep on > >> +working more or less, until userspace stops using the > >> disappeared DRM > >> +device and closes it completely. Userspace will learn of the device > >> +disappearance from the device removed uevent or in some cases > >> specific > >> +ioctls returning EIO. > >> + > >> +This goal raises at least the following requirements for the > >> kernel and > >> +drivers: > >> + > >> +- The kernel must not hang, crash or oops, no matter what > >> userspace was > >> + in the middle of doing when the device disappeared. > >> + > >> +- All GPU jobs that can no longer run must have their fences > >> + force-signalled to avoid inflicting hangs to userspace. > >> + > >> +- KMS connectors must change their status to disconnected. > >> + > >> +- Legacy modesets and pageflips fake success. > >> + > >> +- Atomic commits, both real and TEST_ONLY, fake success. > >> + > >> +- Pending non-blocking KMS operations deliver the DRM events > >> userspace > >> + is expecting. > >> + > >> +- If underlying memory disappears, the mmaps are replaced with > >> harmless > >> + zero pages where access does not raise SIGBUS. Reads return >
[PATCH v3] drm/panfrost: Reduce the amount of logs on deferred probe
There is no point to print deferred probe (and its failures to get resources) as an error. Also there is no need to print regulator errors twice. In case of multiple probe tries this would pollute the dmesg. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Steven Price --- Changes since v2: 1. Rebase 2. Add Steven's review Changes since v1: 1. Remove second error message from calling panfrost_regulator_init() --- drivers/gpu/drm/panfrost/panfrost_device.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 8136babd3ba9..b172087eee6a 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -101,7 +101,9 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev) pfdev->comp->num_supplies, pfdev->regulators); if (ret < 0) { - dev_err(pfdev->dev, "failed to get regulators: %d\n", ret); + if (ret != -EPROBE_DEFER) + dev_err(pfdev->dev, "failed to get regulators: %d\n", + ret); return ret; } @@ -213,10 +215,8 @@ int panfrost_device_init(struct panfrost_device *pfdev) } err = panfrost_regulator_init(pfdev); - if (err) { - dev_err(pfdev->dev, "regulator init failed %d\n", err); + if (err) goto err_out0; - } err = panfrost_reset_init(pfdev); if (err) { -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: device hot-unplug for userspace
Am 27.05.20 um 17:23 schrieb Andrey Grodzovsky: On 5/27/20 10:39 AM, Daniel Vetter wrote: On Wed, May 27, 2020 at 3:51 PM Andrey Grodzovsky wrote: On 5/27/20 2:44 AM, Pekka Paalanen wrote: On Tue, 26 May 2020 10:30:20 -0400 Andrey Grodzovsky wrote: On 5/19/20 6:06 AM, Pekka Paalanen wrote: From: Pekka Paalanen Set up the expectations on how hot-unplugging a DRM device should look like to userspace. Written by Daniel Vetter's request and largely based on his comments in IRC and from https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-May%2F265484.htmldata=02%7C01%7CAndrey.Grodzovsky%40amd.com%7C3c671803b2ba41b2ceac08d8024bcc9a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637261871869742519sdata=ZnhylRubOM0%2BjoreSSYMqVDzZuUdybEsoyBVcTKgxWE%3Dreserved=0 . Signed-off-by: Pekka Paalanen Cc: Daniel Vetter Cc: Andrey Grodzovsky Cc: Dave Airlie Cc: Sean Paul --- Disclaimer: I am a userspace developer writing for other userspace developers. I took some liberties in defining what should happen without knowing what is actually possible or what existing drivers already implement. --- Documentation/gpu/drm-uapi.rst | 75 ++ 1 file changed, 75 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 56fec6ed1ad8..80db4abd2cbd 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -1,3 +1,5 @@ +.. Copyright 2020 DisplayLink (UK) Ltd. + === Userland interfaces === @@ -162,6 +164,79 @@ other hand, a driver requires shared state between clients which is visible to user-space and accessible beyond open-file boundaries, they cannot support render nodes. +Device Hot-Unplug += + +.. note:: + The following is the plan. Implementation is not there yet + (2020 May 13). + +Graphics devices (display and/or render) may be connected via USB (e.g. +display adapters or docking stations) or Thunderbolt (e.g. eGPU). An end +user is able to hot-unplug this kind of devices while they are being +used, and expects that the very least the machine does not crash. Any +damage from hot-unplugging a DRM device needs to be limited as much as +possible and userspace must be given the chance to handle it if it wants +to. Ideally, unplugging a DRM device still lets a desktop to continue +running, but that is going to need explicit support throughout the whole +graphics stack: from kernel and userspace drivers, through display +servers, via window system protocols, and in applications and libraries. + +Other scenarios that should lead to the same are: unrecoverable GPU +crash, PCI device disappearing off the bus, or forced unbind of a driver +from the physical device. + +In other words, from userspace perspective everything needs to keep on +working more or less, until userspace stops using the disappeared DRM +device and closes it completely. Userspace will learn of the device +disappearance from the device removed uevent or in some cases specific +ioctls returning EIO. + +This goal raises at least the following requirements for the kernel and +drivers: + +- The kernel must not hang, crash or oops, no matter what userspace was + in the middle of doing when the device disappeared. + +- All GPU jobs that can no longer run must have their fences + force-signalled to avoid inflicting hangs to userspace. + +- KMS connectors must change their status to disconnected. + +- Legacy modesets and pageflips fake success. + +- Atomic commits, both real and TEST_ONLY, fake success. + +- Pending non-blocking KMS operations deliver the DRM events userspace + is expecting. + +- If underlying memory disappears, the mmaps are replaced with harmless + zero pages where access does not raise SIGBUS. Reads return zeros, + writes are ignored. Regarding this paragraph - what about exiting mappings ? In the first patchset we would actively invalidate all the existing CPU mappings to device memory and i think we still should do it otherwise we will see random crashes in applications as was before. I guess it's because TLBs and page tables are not updated to reflect the fact the device is gone. Hi, I was talking about existing mappings. What I forgot to specify is how new mmap() calls after the device disappearance should work (the end result should be the same still, not failure). I'll clarify this in the next revision. Thanks, pq I see, that ok. Next related question is more for Daniel/Christian - about the implementation of this paragraph, I was thinking about something like checking for device disconnect in ttm_bo_vm_fault_reserved and if so remap the entire VA range for the VMA where the fault address belongs to the global zero page (i.e. (remap_pfn_range(vma, vma->vm_start, page_to_pfn(ZERO_PAGE(vma->vm_start), vma->vm_end - vma->vm_start,
Re: [PATCH v3 055/105] drm/vc4: hvs: Introduce a function to get the assigned FIFO
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > At boot time, if we detect that a pixelvalve has been enabled, we need to > be able to retrieve the HVS channel it has been assigned to so that we can > disable that channel too. Let's create that function that returns the FIFO > or an error from a given output. > > Signed-off-by: Maxime Ripard > --- > +int vc4_hvs_get_fifo_from_output(struct drm_device *dev, unsigned int output) > +{ > + struct vc4_dev *vc4 = to_vc4_dev(dev); > + u32 reg; > + int ret; > + > + switch (output) { > + case 0: > + return 0; > + > + case 1: > + return 1; > + > + case 2: > + reg = HVS_READ(SCALER_DISPECTRL); > + ret = FIELD_GET(SCALER_DISPECTRL_DSP2_MUX_MASK, reg); > + if (ret == 0) > + return 2; > + > + return 0; > + > + case 3: > + reg = HVS_READ(SCALER_DISPCTRL); > + ret = FIELD_GET(SCALER_DISPCTRL_DSP3_MUX_MASK, reg); > + if (ret == 3) > + return -EPIPE; > + > + return ret; > + > + case 4: > + reg = HVS_READ(SCALER_DISPEOLN); > + ret = FIELD_GET(SCALER_DISPEOLN_DSP4_MUX_MASK, reg); > + if (ret == 3) > + return -EPIPE; > + > + return ret; > + > + case 5: > + reg = HVS_READ(SCALER_DISPDITHER); > + ret = FIELD_GET(SCALER_DISPDITHER_DSP5_MUX_MASK, reg); > + if (ret == 3) > + return -EPIPE; Oh, FIELD_GET is new to me. Looks like we should replace VC4_GET_FIELD usage with just using that header, and also VC4_SET_FIELD with WARN_ON(!FIELD_FIT()); FIELD_PREP. Could you follow up with that? Other than that, 54-67 r-b. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #13 from Ilia Mirkin (imir...@alum.mit.edu) --- OK, that looks roughly identical to your v5.3 situation. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #12 from Maurice Gale (mauricega...@gmail.com) --- Created attachment 289357 --> https://bugzilla.kernel.org/attachment.cgi?id=289357=edit Log after firmware installation -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #11 from Maurice Gale (mauricega...@gmail.com) --- Ok, I installed the firmware. Now I have 2/4 displays. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/9] drm/shmem-helpers: Redirect mmap for imported dma-buf
On Wed, May 27, 2020 at 8:32 PM Thomas Zimmermann wrote: > > Hi Daniel, > > what's your plan for this patch set? I'd need this patch for the udl > shmem cleanup. I was pinging some people for a tested-by, I kinda don't want to push this entirely untested. I think at least one of the rendering drivers using shmem would be nice to run this on, I've pinged Rob Herring a bit. -Daniel > > Best regards > Thomas > > Am 11.05.20 um 11:35 schrieb Daniel Vetter: > > Currently this seems to work by converting the sgt into a pages array, > > and then treating it like a native object. Do the right thing and > > redirect mmap to the exporter. > > > > With this nothing is calling get_pages anymore on imported dma-buf, > > and we can start to remove the use of the ->pages array for that case. > > > > v2: Rebase > > > > Cc: Gerd Hoffmann > > Cc: Rob Herring > > Cc: Noralf Trønnes > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > > b/drivers/gpu/drm/drm_gem_shmem_helper.c > > index b9cba5cc61c3..117a7841e284 100644 > > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > > @@ -551,6 +551,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, > > struct vm_area_struct *vma) > > /* Remove the fake offset */ > > vma->vm_pgoff -= drm_vma_node_start(>vma_node); > > > > + if (obj->import_attach) > > + return dma_buf_mmap(obj->dma_buf, vma, 0); > > + > > shmem = to_drm_gem_shmem_obj(obj); > > > > ret = drm_gem_shmem_get_pages(shmem); > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 059/105] drm/vc4: crtc: Add BCM2711 pixelvalves
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > The BCM2711 has 5 pixelvalves, so now that our driver is ready, let's add > support for them. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/vc4/vc4_crtc.c | 84 ++- > drivers/gpu/drm/vc4/vc4_regs.h | 6 +++- > 2 files changed, 88 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c > index 9efd7cb25590..a577ed8f929f 100644 > --- a/drivers/gpu/drm/vc4/vc4_crtc.c > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c > @@ -229,6 +229,13 @@ static u32 vc4_get_fifo_full_level(struct vc4_crtc > *vc4_crtc, u32 format) > case PV_CONTROL_FORMAT_24: > case PV_CONTROL_FORMAT_DSIV_24: > default: > + /* > +* For some reason, the pixelvalve4 doesn't work with > +* the usual formula and will only work with 32. > +*/ > + if (vc4_crtc->data->hvs_output == 5) > + return 32; > + > return fifo_len_bytes - 3 * HVS_FIFO_LATENCY_PIX; > } > } > @@ -237,9 +244,14 @@ static u32 vc4_crtc_get_fifo_full_level_bits(struct > vc4_crtc *vc4_crtc, > u32 format) > { > u32 level = vc4_get_fifo_full_level(vc4_crtc, format); > + u32 ret = 0; > > - return VC4_SET_FIELD(level & 0x3f, > -PV_CONTROL_FIFO_LEVEL); > + if (level > 0x3f) > + ret |= VC4_SET_FIELD((level >> 6) & 0x3, > +PV5_CONTROL_FIFO_LEVEL_HIGH); > + I would drop the conditional here (ORing in zero is fine), and also the & 3 because it would be good to get a warning if you picked a fifo full level that doesn't fit in the field. > + return ret | VC4_SET_FIELD(level & 0x3f, > + PV_CONTROL_FIFO_LEVEL); > } > > /* > @@ -277,6 +289,8 @@ static void vc4_crtc_pixelvalve_reset(struct drm_crtc > *crtc) > > static void vc4_crtc_config_pv(struct drm_crtc *crtc) > { > + struct drm_device *dev = crtc->dev; > + struct vc4_dev *vc4 = to_vc4_dev(dev); > struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc); > struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder); > struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); > @@ -356,6 +370,10 @@ static void vc4_crtc_config_pv(struct drm_crtc *crtc) > if (is_dsi) > CRTC_WRITE(PV_HACT_ACT, mode->hdisplay * pixel_rep); > > + if (vc4->hvs->hvs5) > + CRTC_WRITE(PV_MUX_CFG, > + VC4_SET_FIELD(8, PV_MUX_CFG_RGB_PIXEL_MUX_MODE)); Can we get some #defines in the reg header instead of a magic value? Other than that, r-b. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 006/105] dt-bindings: display: Convert VC4 bindings to schemas
On Wed, May 27, 2020 at 05:47:36PM +0200, Maxime Ripard wrote: > The BCM283x SoCs have a display pipeline composed of several controllers > with device tree bindings that are supported by Linux. > > Now that we have the DT validation in place, let's split into separate > files and convert the device tree bindings for those controllers to > schemas. > > This is just a 1:1 conversion though, and some bindings were incomplete so > it results in example validation warnings that are going to be addressed in > the following patches. > > Cc: Rob Herring > Cc: devicet...@vger.kernel.org > Signed-off-by: Maxime Ripard > --- > Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt | > 174 + > Documentation/devicetree/bindings/display/brcm,bcm2835-dpi.yaml | > 66 +++- > Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml| > 73 ++- > Documentation/devicetree/bindings/display/brcm,bcm2835-hdmi.yaml| > 75 +++- > Documentation/devicetree/bindings/display/brcm,bcm2835-hvs.yaml | > 37 +++- > Documentation/devicetree/bindings/display/brcm,bcm2835-pixelvalve0.yaml | > 40 +- > Documentation/devicetree/bindings/display/brcm,bcm2835-txp.yaml | > 37 +++- > Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml | > 42 +- > Documentation/devicetree/bindings/display/brcm,bcm2835-vc4.yaml | > 34 ++- > Documentation/devicetree/bindings/display/brcm,bcm2835-vec.yaml | > 44 ++- > MAINTAINERS | > 2 +- > 11 files changed, 449 insertions(+), 175 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-dpi.yaml > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-hdmi.yaml > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-hvs.yaml > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-pixelvalve0.yaml > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-txp.yaml > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-v3d.yaml > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-vc4.yaml > create mode 100644 > Documentation/devicetree/bindings/display/brcm,bcm2835-vec.yaml > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml > b/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml > new file mode 100644 > index ..3887675f844e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml > @@ -0,0 +1,73 @@ > +# SPDX-License-Identifier: GPL-2.0 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/brcm,bcm2835-dsi0.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom VC4 (VideoCore4) DSI Controller > + > +maintainers: > + - Eric Anholt > + > +properties: > + compatible: > +enum: > + - brcm,bcm2835-dsi0 > + - brcm,bcm2835-dsi1 > + > + reg: > +maxItems: 1 > + > + clocks: > +items: > + - description: The DSI PLL clock feeding the DSI analog PHY > + - description: The DSI ESC clock > + - description: The DSI pixel clock > + > + clock-output-names: true > +# FIXME: The meta-schemas don't seem to allow it for now > +# items: > +# - description: The DSI byte clock for the PHY > +# - description: The DSI DDR2 clock > +# - description: The DSI DDR clock Doesn't pattern work for you? pattern: '^dsi[0-1]_byte$' Either way, Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Convert get_user_pages() --> pin_user_pages()
On 2020-05-27 01:51, Daniel Vetter wrote: On Wed, May 27, 2020 at 10:48:52AM +0200, Daniel Vetter wrote: On Tue, May 26, 2020 at 03:57:45PM -0700, John Hubbard wrote: On 2020-05-26 14:00, Souptick Joarder wrote: This code was using get_user_pages(), in a "Case 2" scenario (DMA/RDMA), using the categorization from [1]. That means that it's time to convert the get_user_pages() + release_pages() calls to pin_user_pages() + unpin_user_pages() calls. There is some helpful background in [2]: basically, this is a small part of fixing a long-standing disconnect between pinning pages, and file systems' use of those pages. [1] Documentation/core-api/pin_user_pages.rst [2] "Explicit pinning of user-space pages": https://lwn.net/Articles/807108/ I don't think this is a case 2 here, nor is it any of the others. Feels like not covered at all by the doc. radeon has a mmu notifier (might be a bit broken, but hey whatever there's other drivers which have the same concept, but less broken). So when you do an munmap, radeon will release the page refcount. Aha, thanks Daniel. I withdraw my misinformed ACK, then. I forgot to add: It's also not case 3, since there's no hw page fault support. It's all faked in software, and explicitly synchronizes against pending io (or preempts it, that depends a bit upon the jobs running). This is what case 3 was *intended* to cover, but it looks like case 3 needs to be written a little better. I'll attempt that, and Cc you on the actual patch to -mm. (I think we also need a case 5 for an unrelated scenario, too, so it's time.) thanks, -- John Hubbard NVIDIA Which case it that? Note that currently only amdgpu doesn't work like that for gpu dma directly to userspace ranges, it uses hmm and afaiui doens't hold a full page pin refcount. Cheers, Daniel Signed-off-by: Souptick Joarder Cc: John Hubbard Hi, I'm compile tested this, but unable to run-time test, so any testing help is much appriciated. --- drivers/gpu/drm/radeon/radeon_ttm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 5d50c9e..e927de2 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -506,7 +506,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm) uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE; struct page **pages = ttm->pages + pinned; - r = get_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0, + r = pin_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0, pages, NULL); if (r < 0) goto release_pages; @@ -535,7 +535,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm) kfree(ttm->sg); release_pages: - release_pages(ttm->pages, pinned); + unpin_user_pages(ttm->pages, pinned); return r; } @@ -562,7 +562,7 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt *ttm) set_page_dirty(page); Maybe we also need a preceding patch, to fix the above? It should be set_page_dirty_lock(), rather than set_page_dirty(), unless I'm overlooking something (which is very possible!). Either way, from a tunnel vision perspective of changing gup to pup, this looks good to me, so Acked-by: John Hubbard thanks, -- John Hubbard NVIDIA mark_page_accessed(page); - put_page(page); + unpin_user_page(page); } sg_free_table(ttm->sg); -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: pl111: add CONFIG_VEXPRESS_CONFIG dependency
On Wed, May 27, 2020 at 7:50 PM Sam Ravnborg wrote: > On Wed, May 27, 2020 at 05:47:21PM +0200, Arnd Bergmann wrote: > > On Wed, May 27, 2020 at 4:52 PM Sam Ravnborg wrote: > > > > > > Hi Arnd. > > > > > > On Wed, May 27, 2020 at 03:31:42PM +0200, Arnd Bergmann wrote: > > > > The vexpress_config code fails to link in some configurations: > > > > > > > > drivers/gpu/drm/pl111/pl111_versatile.o: in function > > > > `pl111_versatile_init': > > > > (.text+0x1f0): undefined reference to `devm_regmap_init_vexpress_config' > > > > > > > > Add a dependency that links to this only if the dependency is there, > > > > and prevent the configuration where the drm driver is built-in but > > > > the config is a loadable module. > > > > > > > > Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile > > > > init") > > > > Signed-off-by: Arnd Bergmann > > > > > > Could this be another way to fix it: > > > > > > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c > > > b/drivers/gpu/drm/pl111/pl111_versatile.c > > > index 64f01a4e6767..1c38d3bd2e84 100644 > > > --- a/drivers/gpu/drm/pl111/pl111_versatile.c > > > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c > > > @@ -379,7 +379,7 @@ static int pl111_vexpress_clcd_init(struct device > > > *dev, struct device_node *np, > > > u32 val; > > > int ret; > > > > > > - if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG)) > > > + if (!IS_REACHABLE(CONFIG_VEXPRESS_CONFIG)) > > > return -ENODEV; > > > > > > /* > > > > > > > > > Then we no longer have the whole driver depending on > > > the value of VEXPRESS_CONFIG. > > > Not that I like IS_REACHABLE() but we already had > > > IS_ENABLED() to cover up here, and that was not enough. > > > > > > With your patch would we then need the IS_ENABLED() > > > check? > > > > The IS_ENABLED() check is what I'm adding, not removing. I'd still > > the Kconfig dependency combined with that check over > > IS_REACHABLE(), which is more likely to silently not work. > > Then the now redundant IS_ENABLED() check should go. > With you patch it looks like this: > > ... > if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && ...) > pl111_vexpress_clcd_init() > > > And in pl111_vexpress_clcd_init() we have: > > { > if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG)) > return -ENODEV; > > The IS_ENABLED() in pl111_vexpress_clcd_init() is redundant > and the patch should drop it. Ah I see your point now, sorry I missed the double IS_ENABLED() check at first. I'll remove the second one from my patch and resubmit after some more build testing then. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 070/105] drm/vc4: hdmi: rework connectors and encoders
On Wed, May 27, 2020 at 8:51 AM Maxime Ripard wrote: > > the vc4_hdmi driver has some custom structures to hold the data it needs to > associate with the drm_encoder and drm_connector structures. > > However, it allocates them separately from the vc4_hdmi structure which > makes it more complicated than it needs to be. > > Move those structures to be contained by vc4_hdmi and update the code > accordingly. > @@ -1220,7 +1219,7 @@ static int vc4_hdmi_bind(struct device *dev, struct > device *master, void *data) > struct drm_device *drm = dev_get_drvdata(master); > struct vc4_dev *vc4 = drm->dev_private; > struct vc4_hdmi *hdmi; > - struct vc4_hdmi_encoder *vc4_hdmi_encoder; > + struct drm_encoder *encoder; > struct device_node *ddc_node; > u32 value; > int ret; > @@ -1229,14 +1228,10 @@ static int vc4_hdmi_bind(struct device *dev, struct > device *master, void *data) > if (!hdmi) > return -ENOMEM; > > - vc4_hdmi_encoder = devm_kzalloc(dev, sizeof(*vc4_hdmi_encoder), > - GFP_KERNEL); > - if (!vc4_hdmi_encoder) > - return -ENOMEM; > - vc4_hdmi_encoder->base.type = VC4_ENCODER_TYPE_HDMI0; > - hdmi->encoder = _hdmi_encoder->base.base; > - > hdmi->pdev = pdev; > + encoder = >encoder.base.base; > + encoder->base.type = VC4_ENCODER_TYPE_HDMI0; Wait, does this patch build? setting struct drm_encoder->base.type = VC4_* seems very wrong, when previously we were setting struct vc4_hdmi_encoder->base.type (struct vc4_encoder->type). Other than this, patch 68-78 r-b. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/9] drm/shmem-helpers: Redirect mmap for imported dma-buf
Hi Daniel, what's your plan for this patch set? I'd need this patch for the udl shmem cleanup. Best regards Thomas Am 11.05.20 um 11:35 schrieb Daniel Vetter: > Currently this seems to work by converting the sgt into a pages array, > and then treating it like a native object. Do the right thing and > redirect mmap to the exporter. > > With this nothing is calling get_pages anymore on imported dma-buf, > and we can start to remove the use of the ->pages array for that case. > > v2: Rebase > > Cc: Gerd Hoffmann > Cc: Rob Herring > Cc: Noralf Trønnes > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c > b/drivers/gpu/drm/drm_gem_shmem_helper.c > index b9cba5cc61c3..117a7841e284 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -551,6 +551,9 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct > vm_area_struct *vma) > /* Remove the fake offset */ > vma->vm_pgoff -= drm_vma_node_start(>vma_node); > > + if (obj->import_attach) > + return dma_buf_mmap(obj->dma_buf, vma, 0); > + > shmem = to_drm_gem_shmem_obj(obj); > > ret = drm_gem_shmem_get_pages(shmem); > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 041/105] drm/vc4: crtc: Move HVS mode config to HVS file
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/vc4/vc4_crtc.c | 272 +--- > drivers/gpu/drm/vc4/vc4_drv.h | 5 +- > drivers/gpu/drm/vc4/vc4_hvs.c | 298 ++- > 3 files changed, 309 insertions(+), 266 deletions(-) > static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc) > { > - struct drm_device *dev = crtc->dev; > - struct vc4_dev *vc4 = to_vc4_dev(dev); > struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); > struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state); > - struct drm_display_mode *mode = >state->adjusted_mode; > - bool interlace = mode->flags & DRM_MODE_FLAG_INTERLACE; > bool debug_dump_regs = false; > > if (debug_dump_regs) { > @@ -418,42 +372,10 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc > *crtc) > drm_print_regset32(, _crtc->regset); > } > > - if (vc4_crtc->data->hvs_output == 2) { > - u32 dispctrl; > - u32 dsp3_mux; > - > - /* > -* SCALER_DISPCTRL_DSP3 = X, where X < 2 means 'connect DSP3 > to > -* FIFO X'. > -* SCALER_DISPCTRL_DSP3 = 3 means 'disable DSP 3'. > -* > -* DSP3 is connected to FIFO2 unless the transposer is > -* enabled. In this case, FIFO 2 is directly accessed by the > -* TXP IP, and we need to disable the FIFO2 -> pixelvalve1 > -* route. > -*/ > - if (vc4_state->feed_txp) > - dsp3_mux = VC4_SET_FIELD(3, SCALER_DISPCTRL_DSP3_MUX); > - else > - dsp3_mux = VC4_SET_FIELD(2, SCALER_DISPCTRL_DSP3_MUX); > - > - dispctrl = HVS_READ(SCALER_DISPCTRL) & > - ~SCALER_DISPCTRL_DSP3_MUX_MASK; > - HVS_WRITE(SCALER_DISPCTRL, dispctrl | dsp3_mux); > - } I just noticed, this block being moved looks like it should probably have been removed as part of patch #33. Cleaning this up I think will impact the following patches. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 2/2] drm: xlnx: driver for Xilinx DSI TX Subsystem
Hi Laurent, On Sat, 2020-05-23 at 20:08:13 -0700, Laurent Pinchart wrote: > Hi GVRao, > > Thank you for the patch. > > On Mon, May 04, 2020 at 11:43:48AM -0700, Hyun Kwon wrote: > > On Mon, 2020-04-20 at 14:20:56 -0700, Venkateshwar Rao Gannavarapu wrote: > > > The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video > > > data from AXI-4 stream interface. > > > > > > It supports upto 4 lanes, optional register interface for the DPHY, > > > > I don't see the register interface for dphy support. > > I think the D-PHY should be supported through a PHY driver, as it seems > to be shared between different subsystems. > Right, if the logic is shared across subsystems. I can't tell if that's the case as the IP comes as a single block. Maybe GVRao can confirm. > > > multiple RGB color formats, command mode and video mode. > > > This is a MIPI-DSI host driver and provides DSI bus for panels. > > > This driver also helps to communicate with its panel using panel > > > framework. > > > > > > Signed-off-by: Venkateshwar Rao Gannavarapu > > > > > > --- > > > drivers/gpu/drm/xlnx/Kconfig| 11 + > > > drivers/gpu/drm/xlnx/Makefile | 2 + > > > drivers/gpu/drm/xlnx/xlnx_dsi.c | 755 > > > > > Daniel Vetter has recently expressed his opiion that bridge drivers > should go to drivers/gpu/drm/bridge/. It would then be > drivers/gpu/drm/bridge/xlnx/. I don't have a strong opinion myself. > > > > 3 files changed, 768 insertions(+) > > > create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c > > > > > > diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig > > > index aa6cd88..73873cf 100644 > > > --- a/drivers/gpu/drm/xlnx/Kconfig > > > +++ b/drivers/gpu/drm/xlnx/Kconfig > > > @@ -11,3 +11,14 @@ config DRM_ZYNQMP_DPSUB > > > This is a DRM/KMS driver for ZynqMP DisplayPort controller. Choose > > > this option if you have a Xilinx ZynqMP SoC with DisplayPort > > > subsystem. > > > + > > > +config DRM_XLNX_DSI > > > +tristate "Xilinx DRM DSI Subsystem Driver" > > > +select DRM_MIPI_DSI > > > +select DRM_PANEL > > > +select DRM_PANEL_SIMPLE > > > +help > > > + This enables support for Xilinx MIPI-DSI. > > > > This sentence is not needed with below. Could you please rephrase the whole? > > > > > + This is a DRM/KMS driver for Xilinx programmable DSI controller. > > > + Choose this option if you have a Xilinx MIPI DSI-TX controller > > > + subsytem. > > > > These seem incorrectly indented. > > > > > diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile > > > index 2b844c6..b7ee6ef 100644 > > > --- a/drivers/gpu/drm/xlnx/Makefile > > > +++ b/drivers/gpu/drm/xlnx/Makefile > > > @@ -1,2 +1,4 @@ > > > zynqmp-dpsub-objs += zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o > > > obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o > > > + > > > +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o > > > diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c > > > b/drivers/gpu/drm/xlnx/xlnx_dsi.c > > > new file mode 100644 > > > index 000..b8cae59 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c > > > @@ -0,0 +1,755 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Xilinx FPGA MIPI DSI Tx Controller driver > > > + * > > > + * Copyright (C) 2017 - 2019 Xilinx, Inc. > > > + * > > > + * Authors: > > > + * - Saurabh Sengar > > > + * - Venkateshwar Rao Gannavarapu > > > > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#include > > > +#include > > > + > > > +/* DSI Tx IP registers */ > > > +#define XDSI_CCR 0x00 > > > +#define XDSI_CCR_COREENB BIT(0) > > > +#define XDSI_CCR_SOFTRST BIT(1) > > > +#define XDSI_CCR_CRREADY BIT(2) > > > +#define XDSI_CCR_CMDMODE BIT(3) > > > +#define XDSI_CCR_DFIFORSTBIT(4) > > > +#define XDSI_CCR_CMDFIFORST BIT(5) > > > +#define XDSI_PCR 0x04 [snip] > > > + } > > > + > > > + ret = clk_prepare_enable(dsi->video_aclk); > > > + if (ret) { > > > + dev_err(dev, "failed to enable video clk %d\n", ret); > > > + goto err_disable_dphy_clk; > > > + } > > > + > > > + ret = component_add(dev, _dsi_component_ops); > > The driver should expose the DSI-TX as a drm_bridge instead of using the > component framework. You shouldn't register a drm_encoder, and I don't > think you should register a drm_connector either. Only bridge operations > should be exposed, and the drm_bridge .attach() operation should return > an error when DRM_BRIDGE_ATTACH_NO_CONNECTOR is not set. The top-level > driver using this bridge should create the drm_encoder and >
Re: [PATCH] drm: pl111: add CONFIG_VEXPRESS_CONFIG dependency
Hi Arnd. On Wed, May 27, 2020 at 05:47:21PM +0200, Arnd Bergmann wrote: > On Wed, May 27, 2020 at 4:52 PM Sam Ravnborg wrote: > > > > Hi Arnd. > > > > On Wed, May 27, 2020 at 03:31:42PM +0200, Arnd Bergmann wrote: > > > The vexpress_config code fails to link in some configurations: > > > > > > drivers/gpu/drm/pl111/pl111_versatile.o: in function > > > `pl111_versatile_init': > > > (.text+0x1f0): undefined reference to `devm_regmap_init_vexpress_config' > > > > > > Add a dependency that links to this only if the dependency is there, > > > and prevent the configuration where the drm driver is built-in but > > > the config is a loadable module. > > > > > > Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile > > > init") > > > Signed-off-by: Arnd Bergmann > > > > Could this be another way to fix it: > > > > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c > > b/drivers/gpu/drm/pl111/pl111_versatile.c > > index 64f01a4e6767..1c38d3bd2e84 100644 > > --- a/drivers/gpu/drm/pl111/pl111_versatile.c > > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c > > @@ -379,7 +379,7 @@ static int pl111_vexpress_clcd_init(struct device *dev, > > struct device_node *np, > > u32 val; > > int ret; > > > > - if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG)) > > + if (!IS_REACHABLE(CONFIG_VEXPRESS_CONFIG)) > > return -ENODEV; > > > > /* > > > > > > Then we no longer have the whole driver depending on > > the value of VEXPRESS_CONFIG. > > Not that I like IS_REACHABLE() but we already had > > IS_ENABLED() to cover up here, and that was not enough. > > > > With your patch would we then need the IS_ENABLED() > > check? > > The IS_ENABLED() check is what I'm adding, not removing. I'd still > the Kconfig dependency combined with that check over > IS_REACHABLE(), which is more likely to silently not work. Then the now redundant IS_ENABLED() check should go. With you patch it looks like this: ... if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && ...) pl111_vexpress_clcd_init() And in pl111_vexpress_clcd_init() we have: { if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG)) return -ENODEV; The IS_ENABLED() in pl111_vexpress_clcd_init() is redundant and the patch should drop it. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: use drm_dev_has_vblank more
On Wed, May 27, 2020 at 01:11:34PM +0200, Daniel Vetter wrote: > For historical reasons it's called dev->num_crtcs, which is rather > confusing ever since kms was added. But now we have a nice helper, so > let's use it for better readability! > > Only code change is in atomic helpers: vblank support means that > dev->irq_enabled must be set too. Another one of these quirky things > ... But since it's implied we can simplify that check. > > Signed-off-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter Acked-by: Sam Ravnborg > --- > drivers/gpu/drm/drm_atomic_helper.c | 2 +- > drivers/gpu/drm/drm_irq.c | 2 +- > drivers/gpu/drm/drm_vblank.c| 14 +++--- > 3 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 0a541368246e..bfcc7857a9a1 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1097,7 +1097,7 @@ disable_outputs(struct drm_device *dev, struct > drm_atomic_state *old_state) > else if (funcs->dpms) > funcs->dpms(crtc, DRM_MODE_DPMS_OFF); > > - if (!(dev->irq_enabled && dev->num_crtcs)) > + if (!drm_dev_has_vblank(dev)) > continue; > > ret = drm_crtc_vblank_get(crtc); > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 588be45abd7a..09d6e9e2e075 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -181,7 +181,7 @@ int drm_irq_uninstall(struct drm_device *dev) >* vblank/irq handling. KMS drivers must ensure that vblanks are all >* disabled when uninstalling the irq handler. >*/ > - if (dev->num_crtcs) { > + if (drm_dev_has_vblank(dev)) { > spin_lock_irqsave(>vbl_lock, irqflags); > for (i = 0; i < dev->num_crtcs; i++) { > struct drm_vblank_crtc *vblank = >vblank[i]; > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index e278d6407f8e..162d9f7e692a 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -605,7 +605,7 @@ void drm_calc_timestamping_constants(struct drm_crtc > *crtc, > int linedur_ns = 0, framedur_ns = 0; > int dotclock = mode->crtc_clock; > > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return; > > if (WARN_ON(pipe >= dev->num_crtcs)) > @@ -1065,7 +1065,7 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc, > unsigned int pipe = drm_crtc_index(crtc); > ktime_t now; > > - if (dev->num_crtcs > 0) { > + if (drm_dev_has_vblank(dev)) { > seq = drm_vblank_count_and_time(dev, pipe, ); > } else { > seq = 0; > @@ -1137,7 +1137,7 @@ static int drm_vblank_get(struct drm_device *dev, > unsigned int pipe) > unsigned long irqflags; > int ret = 0; > > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return -EINVAL; > > if (WARN_ON(pipe >= dev->num_crtcs)) > @@ -1506,7 +1506,7 @@ static void drm_legacy_vblank_pre_modeset(struct > drm_device *dev, > struct drm_vblank_crtc *vblank = >vblank[pipe]; > > /* vblank is not initialized (IRQ not installed ?), or has been freed */ > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return; > > if (WARN_ON(pipe >= dev->num_crtcs)) > @@ -1533,7 +1533,7 @@ static void drm_legacy_vblank_post_modeset(struct > drm_device *dev, > unsigned long irqflags; > > /* vblank is not initialized (IRQ not installed ?), or has been freed */ > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return; > > if (WARN_ON(pipe >= dev->num_crtcs)) > @@ -1558,7 +1558,7 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device > *dev, void *data, > unsigned int pipe; > > /* If drm_vblank_init() hasn't been called yet, just no-op */ > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return 0; > > /* KMS drivers handle this internally */ > @@ -1896,7 +1896,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned > int pipe) > unsigned long irqflags; > bool disable_irq, fence_cookie; > > - if (WARN_ON_ONCE(!dev->num_crtcs)) > + if (WARN_ON_ONCE(!drm_dev_has_vblank(dev))) > return false; > > if (WARN_ON(pipe >= dev->num_crtcs)) > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 040/105] drm/vc4: crtc: Turn pixelvalve reset into a function
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > The driver resets the pixelvalve FIFO in a number of occurences without > always using the same sequence. > > Since this will be critical for BCM2711, let's move that sequence to a > function so that we are consistent. > > Signed-off-by: Maxime Ripard Patch 34-40 also r-b. Going to take a little break, this is a lot to try to process at once. Hopefully you can merge reviewed stuff to drm-misc and shorten the series. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 033/105] drm/vc4: crtc: Assign output to channel automatically
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > The HVS found in the BCM2711 has 6 outputs and 3 FIFOs, with each output > being connected to a pixelvalve, and some muxing between the FIFOs and > outputs. > > Any output cannot feed from any FIFO though, and they all have a bunch of > constraints. > > In order to support this, let's store the possible FIFOs each output can be > assigned to in the vc4_crtc_data, and use that information at atomic_check > time to iterate over all the CRTCs enabled and assign them FIFOs. > > The channel assigned is then set in the vc4_crtc_state so that the rest of > the driver can use it. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/vc4/vc4_crtc.c | 37 + > drivers/gpu/drm/vc4/vc4_drv.h | 7 +- > drivers/gpu/drm/vc4/vc4_kms.c | 142 -- > drivers/gpu/drm/vc4/vc4_regs.h | 10 ++- > 4 files changed, 172 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c > index 580b37ad514d..a6c3f2f907bd 100644 > --- a/drivers/gpu/drm/vc4/vc4_crtc.c > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c > @@ -88,6 +88,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc > *crtc, > struct drm_device *dev = crtc->dev; > struct vc4_dev *vc4 = to_vc4_dev(dev); > struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); > + struct vc4_crtc_state *vc4_crtc_state = > to_vc4_crtc_state(crtc->state); > unsigned int cob_size; > u32 val; > int fifo_lines; > @@ -104,7 +105,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc > *crtc, > * Read vertical scanline which is currently composed for our > * pixelvalve by the HVS, and also the scaler status. > */ > - val = HVS_READ(SCALER_DISPSTATX(vc4_crtc->channel)); > + val = HVS_READ(SCALER_DISPSTATX(vc4_crtc_state->assigned_channel)); > > /* Get optional system timestamp after query. */ > if (etime) > @@ -124,7 +125,7 @@ static bool vc4_crtc_get_scanout_position(struct drm_crtc > *crtc, > *hpos += mode->crtc_htotal / 2; > } > > - cob_size = vc4_crtc_get_cob_allocation(vc4, vc4_crtc->channel); > + cob_size = vc4_crtc_get_cob_allocation(vc4, > vc4_crtc_state->assigned_channel); > /* This is the offset we need for translating hvs -> pv scanout pos. > */ > fifo_lines = cob_size / mode->crtc_hdisplay; > > @@ -211,6 +212,7 @@ vc4_crtc_lut_load(struct drm_crtc *crtc) > struct drm_device *dev = crtc->dev; > struct vc4_dev *vc4 = to_vc4_dev(dev); > struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); > + struct vc4_crtc_state *vc4_crtc_state = > to_vc4_crtc_state(crtc->state); > u32 i; > > /* The LUT memory is laid out with each HVS channel in order, > @@ -219,7 +221,7 @@ vc4_crtc_lut_load(struct drm_crtc *crtc) > */ > HVS_WRITE(SCALER_GAMADDR, > SCALER_GAMADDR_AUTOINC | > - (vc4_crtc->channel * 3 * crtc->gamma_size)); > + (vc4_crtc_state->assigned_channel * 3 * crtc->gamma_size)); > > for (i = 0; i < crtc->gamma_size; i++) > HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_r[i]); > @@ -392,7 +394,7 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc) > drm_print_regset32(, _crtc->regset); > } > > - if (vc4_crtc->channel == 2) { > + if (vc4_crtc->data->hvs_output == 2) { > u32 dispctrl; > u32 dsp3_mux; Looks like this hunk is maybe supposed to be in the hvs_output rename patch? > @@ -419,7 +421,7 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc) > if (!vc4_state->feed_txp) > vc4_crtc_config_pv(crtc); > > - HVS_WRITE(SCALER_DISPBKGNDX(vc4_crtc->channel), > + HVS_WRITE(SCALER_DISPBKGNDX(vc4_state->assigned_channel), > SCALER_DISPBKGND_AUTOHS | > SCALER_DISPBKGND_GAMMA | > (interlace ? SCALER_DISPBKGND_INTERLACE : 0)); > @@ -451,7 +453,8 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc, > struct drm_device *dev = crtc->dev; > struct vc4_dev *vc4 = to_vc4_dev(dev); > struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc); > - u32 chan = vc4_crtc->channel; > + struct vc4_crtc_state *vc4_crtc_state = to_vc4_crtc_state(old_state); > + u32 chan = vc4_crtc_state->assigned_channel; > int ret; > require_hvs_enabled(dev); > > @@ -530,12 +533,12 @@ static void vc4_crtc_update_dlist(struct drm_crtc *crtc) > crtc->state->event = NULL; > } > > - HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel), > + HVS_WRITE(SCALER_DISPLISTX(vc4_state->assigned_channel), > vc4_state->mm.start); > > spin_unlock_irqrestore(>event_lock, flags); >
Re: [PATCH v3 032/105] drm/vc4: crtc: Enable and disable the PV in atomic_enable / disable
On Wed, May 27, 2020 at 8:50 AM Maxime Ripard wrote: > > The VIDEN bit in the pixelvalve currently being used to enable or disable > the pixelvalve seems to not be enough in some situations, which whill end > up with the pixelvalve stalling. > > In such a case, even re-enabling VIDEN doesn't bring it back and we need to > clear the FIFO. This can only be done if the pixelvalve is disabled though. > > In order to overcome this, we can configure the pixelvalve during > mode_set_no_fb, but only enable it in atomic_enable and flush the FIFO > there, and in atomic_disable disable the pixelvalve again. What displays has this been tested with? Getting this sequencing right is so painful, and things like DSI are tricky to get to light up. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 020/105] drm/vc4: plane: Create overlays for any CRTC
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote: > > Now that we have everything in place, we can now register all the overlay > planes that can be assigned to all the CRTCs. > > This has two side effects: > > - The number of overlay planes is reduced from 24 to 8. This is temporary > and will be increased again in the next patch. > > - The ID of the various planes is changed again, and we will now have all > the primary planes, then all the overlay planes and finally the cursor > planes. This shouldn't cause any issue since the ordering between > primary, overlay and cursor planes is preserved. > > Signed-off-by: Maxime Ripard Honestly, I'd squash this with the previous two patches, the individual refactors don't make much sense on their own or simplify this patch I think. Either way, patch 17-29 r-b. > --- > drivers/gpu/drm/vc4/vc4_plane.c | 35 +- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index 824c188980b0..5335123ae2a0 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -1378,26 +1378,27 @@ int vc4_plane_create_additional_planes(struct > drm_device *drm) > struct drm_crtc *crtc; > unsigned int i; > > - drm_for_each_crtc(crtc, drm) { > - /* Set up some arbitrary number of planes. We're not limited > -* by a set number of physical registers, just the space in > -* the HVS (16k) and how small an plane can be (28 bytes). > -* However, each plane we set up takes up some memory, and > -* increases the cost of looping over planes, which atomic > -* modesetting does quite a bit. As a result, we pick a > -* modest number of planes to expose, that should hopefully > -* still cover any sane usecase. > -*/ > - for (i = 0; i < 8; i++) { > - struct drm_plane *plane = > - vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY); > + /* Set up some arbitrary number of planes. We're not limited > +* by a set number of physical registers, just the space in > +* the HVS (16k) and how small an plane can be (28 bytes). > +* However, each plane we set up takes up some memory, and > +* increases the cost of looping over planes, which atomic > +* modesetting does quite a bit. As a result, we pick a > +* modest number of planes to expose, that should hopefully > +* still cover any sane usecase. > +*/ > + for (i = 0; i < 8; i++) { > + struct drm_plane *plane = > + vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY); > > - if (IS_ERR(plane)) > - continue; > + if (IS_ERR(plane)) > + continue; > > - plane->possible_crtcs = drm_crtc_mask(crtc); > - } > + plane->possible_crtcs = > + GENMASK(drm->mode_config.num_crtc - 1, 0); > + } > > + drm_for_each_crtc(crtc, drm) { > /* Set up the legacy cursor after overlay initialization, > * since we overlay planes on the CRTC in the order they were > * initialized. > -- > git-series 0.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 016/105] drm/vc4: plane: Improve LBM usage
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote: > > From: Dave Stevenson > > LBM allocations were always taking the worst case sizing of > max(src_width, dst_width) * 16. This is significantly over > the required sizing, and stops us rendering multiple 4k images > to the screen. > > Add some of the additional constraints to more accurately > describe the LBM requirements. > > Signed-off-by: Dave Stevenson > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/vc4/vc4_plane.c | 31 --- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index 1575c05e3106..602927745f84 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -142,9 +142,10 @@ static const struct hvs_format *vc4_get_hvs_format(u32 > drm_format) > return NULL; > } > > -static enum vc4_scaling_mode vc4_get_scaling_mode(u32 src, u32 dst) > +static enum vc4_scaling_mode vc4_get_scaling_mode(u32 src, u32 dst, > + bool chroma_vrep) > { > - if (dst == src) > + if (dst == src && !chroma_vrep) > return VC4_SCALING_NONE; > if (3 * dst >= 2 * src) > return VC4_SCALING_PPF; > @@ -369,9 +370,11 @@ static int vc4_plane_setup_clipping_and_scaling(struct > drm_plane_state *state) > return ret; > > vc4_state->x_scaling[0] = vc4_get_scaling_mode(vc4_state->src_w[0], > - vc4_state->crtc_w); > + vc4_state->crtc_w, > + false); > vc4_state->y_scaling[0] = vc4_get_scaling_mode(vc4_state->src_h[0], > - vc4_state->crtc_h); > + vc4_state->crtc_h, > + false); > > vc4_state->is_unity = (vc4_state->x_scaling[0] == VC4_SCALING_NONE && >vc4_state->y_scaling[0] == VC4_SCALING_NONE); > @@ -384,10 +387,12 @@ static int vc4_plane_setup_clipping_and_scaling(struct > drm_plane_state *state) > > vc4_state->x_scaling[1] = > vc4_get_scaling_mode(vc4_state->src_w[1], > -vc4_state->crtc_w); > +vc4_state->crtc_w, > +v_subsample == 2); > vc4_state->y_scaling[1] = > vc4_get_scaling_mode(vc4_state->src_h[1], > -vc4_state->crtc_h); > +vc4_state->crtc_h, > +v_subsample == 2); > > /* YUV conversion requires that horizontal scaling be enabled > * on the UV plane even if vc4_get_scaling_mode() returned The change above isn't mentioned in the commit message and I don't understand what's going on. It should be split out with an explanation. > @@ -437,10 +442,7 @@ static void vc4_write_ppf(struct vc4_plane_state > *vc4_state, u32 src, u32 dst) > static u32 vc4_lbm_size(struct drm_plane_state *state) > { > struct vc4_plane_state *vc4_state = to_vc4_plane_state(state); > - /* This is the worst case number. One of the two sizes will > -* be used depending on the scaling configuration. > -*/ > - u32 pix_per_line = max(vc4_state->src_w[0], (u32)vc4_state->crtc_w); > + u32 pix_per_line; > u32 lbm; > > /* LBM is not needed when there's no vertical scaling. */ > @@ -448,6 +450,11 @@ static u32 vc4_lbm_size(struct drm_plane_state *state) > vc4_state->y_scaling[1] == VC4_SCALING_NONE) > return 0; > > + if (vc4_state->x_scaling[0] == VC4_SCALING_TPZ) > + pix_per_line = vc4_state->crtc_w; > + else > + pix_per_line = vc4_state->src_w[0]; Looks like it's also crtc_w for RGB or 4:4:4 and HPPF in (0.5,1.0]. Maybe drop a note in here that we're not covering that case, but src_w > crtc_w so it's safe at least. > + > if (!vc4_state->is_yuv) { > if (vc4_state->y_scaling[0] == VC4_SCALING_TPZ) > lbm = pix_per_line * 8; > @@ -583,7 +590,9 @@ static int vc4_plane_allocate_lbm(struct drm_plane_state > *state) > spin_lock_irqsave(>hvs->mm_lock, irqflags); > ret = drm_mm_insert_node_generic(>hvs->lbm_mm, > _state->lbm, > -lbm_size, 32, 0, 0); > +lbm_size, > +vc4->hvs->hvs5 ? 64 : 32, > +
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #10 from Ilia Mirkin (imir...@alum.mit.edu) --- It looks like you don't have firmware present, which starting with v5.6 fails the whole module load. (I believe that's a separate bug.) Try installing linux-firmware, and making the firmware available in /lib/firmware at nouveau module load time (so if it's loaded from initrd, it needs to be in initrd). -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 015/105] drm/vc4: hvs: Boost the core clock during modeset
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote: > > In order to prevent timeouts and stalls in the pipeline, the core clock > needs to be maxed at 500MHz during a modeset on the BCM2711. Like, the whole system's core clock? How is it reasonable for some device driver to crank the system's core clock up and back down to some fixed-in-the-driver frequency? Sounds like you need some sort of opp thing here. Patch 13,14 r-b. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 012/105] drm/vc4: drv: Support BCM2711
On Wed, May 27, 2020 at 8:49 AM Maxime Ripard wrote: > > The BCM2711 has a reworked display pipeline, and the load tracker needs > some adjustement to operate properly. Let's add a compatible for BCM2711 > and disable the load tracker until properly supported. > > Signed-off-by: Maxime Ripard > --- > drivers/gpu/drm/vc4/vc4_drv.c | 1 +- > drivers/gpu/drm/vc4/vc4_drv.h | 3 ++- > drivers/gpu/drm/vc4/vc4_kms.c | 42 +++--- > drivers/gpu/drm/vc4/vc4_plane.c | 5 - > 4 files changed, 38 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c > index 76f93b662766..d7f554a6f0ed 100644 > --- a/drivers/gpu/drm/vc4/vc4_drv.c > +++ b/drivers/gpu/drm/vc4/vc4_drv.c > @@ -364,6 +364,7 @@ static int vc4_platform_drm_remove(struct platform_device > *pdev) > } > > static const struct of_device_id vc4_of_match[] = { > + { .compatible = "brcm,bcm2711-vc5", }, > { .compatible = "brcm,bcm2835-vc4", }, > { .compatible = "brcm,cygnus-vc4", }, > {}, Patch 6 Acked-by: Eric Anholt Patch 7-11 Reviewed-by: Eric Anholt This one to start probing needs to move later in the series once the vc5 support is actually present in the driver. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 002/105] reset: simple: Add reset callback
Hi Maxime, On Wed, 2020-05-27 at 17:47 +0200, Maxime Ripard wrote: > The reset-simple code lacks a reset callback that is still pretty easy to > implement. The only real thing to consider is the delay needed for a device > to be reset, so let's expose that as part of the reset-simple driver data. > > Cc: Philipp Zabel > Reviewed-by: Philipp Zabel > Signed-off-by: Maxime Ripard Thank you, I've applied patches 1 & 2 to the reset/next branch. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] [v2] drm/panfrost: Fix runtime PM imbalance on error
On 22/05/2020 14:41, Dinghao Liu wrote: The caller expects panfrost_job_hw_submit() to increase runtime PM usage counter. The refcount decrement on the error branch of WARN_ON() will break the counter balance and needs to be removed. Signed-off-by: Dinghao Liu Reviewed-by: Steven Price Thanks, Steve --- Changelog: v2: - Remove refcount decrement on the error path of WARN_ON() rather than add refcount decrement on the error path of pm_runtime_get_sync(). --- drivers/gpu/drm/panfrost/panfrost_job.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 7914b1570841..1092d9754f0f 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -150,7 +150,6 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) return; if (WARN_ON(job_read(pfdev, JS_COMMAND_NEXT(js { - pm_runtime_put_sync_autosuspend(pfdev->dev); return; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/i915/selftests: avoid bogus maybe-uninitialized warning
Quoting Arnd Bergmann (2020-05-27 15:05:10) > gcc has a problem following values through IS_ERR/PTR_ERR macros, > leading to a false-positive warning in allmodconfig, with any > compiler version: > > In file included from drivers/gpu/drm/i915/gt/intel_lrc.c:5892: > drivers/gpu/drm/i915/gt/selftest_lrc.c: In function > 'create_gpr_client.isra.0': > drivers/gpu/drm/i915/gt/selftest_lrc.c:2902:23: error: 'rq' may be used > uninitialized in this function [-Werror=maybe-uninitialized] > > This one is hard to avoid without impairing readability or adding a > bugus NULL pointer. > > Fixes: c92724de6db1 ("drm/i915/selftests: Try to detect rollback during > batchbuffer preemption") > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/drm/i915/gt/selftest_lrc.c | 21 + > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c > b/drivers/gpu/drm/i915/gt/selftest_lrc.c > index 824f99c4cc7c..933c3f5adf0a 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c > @@ -2908,23 +2908,25 @@ create_gpr_client(struct intel_engine_cs *engine, > > vma = i915_vma_instance(global->obj, ce->vm, NULL); > if (IS_ERR(vma)) { > - err = PTR_ERR(vma); > + rq = ERR_CAST(vma); > goto out_ce; > } > > err = i915_vma_pin(vma, 0, 0, PIN_USER); > - if (err) > + if (err) { > + rq = ERR_PTR(err); > goto out_ce; > + } > > batch = create_gpr_user(engine, vma, offset); > if (IS_ERR(batch)) { > - err = PTR_ERR(batch); > + rq = ERR_CAST(batch); > goto out_vma; > } > > rq = intel_context_create_request(ce); > if (IS_ERR(rq)) { > - err = PTR_ERR(rq); > + rq = ERR_CAST(rq); > goto out_batch; > } > > @@ -2946,17 +2948,20 @@ create_gpr_client(struct intel_engine_cs *engine, > i915_vma_unlock(batch); > i915_vma_unpin(batch); > > - if (!err) > + if (!err) { > i915_request_get(rq); > - i915_request_add(rq); > - > + i915_request_add(rq); > + } else { > + i915_request_add(rq); > + rq = ERR_PTR(err); > + } > out_batch: > i915_vma_put(batch); > out_vma: > i915_vma_unpin(vma); > out_ce: > intel_context_put(ce); > - return err ? ERR_PTR(err) : rq; > + return rq; Hmm, I've used this style a few times, so could do with some syntactic refinement. drivers/gpu/drm/i915/gem/i915_gem_userptr.c:return err ? ERR_PTR(err) : mm->mn; drivers/gpu/drm/i915/gt/selftest_hangcheck.c: return err ? ERR_PTR(err) : rq; drivers/gpu/drm/i915/gt/selftest_lrc.c: return err ? ERR_PTR(err) : rq; drivers/gpu/drm/i915/selftests/i915_gem_gtt.c: return err ? ERR_PTR(err) : rq; drivers/gpu/drm/i915/selftests/i915_request.c: return err ? ERR_PTR(err) : request; drivers/gpu/drm/i915/selftests/igt_spinner.c: return err ? ERR_PTR(err) : rq; -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: pl111: add CONFIG_VEXPRESS_CONFIG dependency
On Wed, May 27, 2020 at 4:52 PM Sam Ravnborg wrote: > > Hi Arnd. > > On Wed, May 27, 2020 at 03:31:42PM +0200, Arnd Bergmann wrote: > > The vexpress_config code fails to link in some configurations: > > > > drivers/gpu/drm/pl111/pl111_versatile.o: in function `pl111_versatile_init': > > (.text+0x1f0): undefined reference to `devm_regmap_init_vexpress_config' > > > > Add a dependency that links to this only if the dependency is there, > > and prevent the configuration where the drm driver is built-in but > > the config is a loadable module. > > > > Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile init") > > Signed-off-by: Arnd Bergmann > > Could this be another way to fix it: > > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c > b/drivers/gpu/drm/pl111/pl111_versatile.c > index 64f01a4e6767..1c38d3bd2e84 100644 > --- a/drivers/gpu/drm/pl111/pl111_versatile.c > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c > @@ -379,7 +379,7 @@ static int pl111_vexpress_clcd_init(struct device *dev, > struct device_node *np, > u32 val; > int ret; > > - if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG)) > + if (!IS_REACHABLE(CONFIG_VEXPRESS_CONFIG)) > return -ENODEV; > > /* > > > Then we no longer have the whole driver depending on > the value of VEXPRESS_CONFIG. > Not that I like IS_REACHABLE() but we already had > IS_ENABLED() to cover up here, and that was not enough. > > With your patch would we then need the IS_ENABLED() > check? The IS_ENABLED() check is what I'm adding, not removing. I'd still the Kconfig dependency combined with that check over IS_REACHABLE(), which is more likely to silently not work. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/i915: work around false-positive maybe-uninitialized warning
Quoting Arnd Bergmann (2020-05-27 15:05:09) > gcc-9 gets confused by the code flow in check_dirty_whitelist: > > drivers/gpu/drm/i915/gt/selftest_workarounds.c: In function > 'check_dirty_whitelist': > drivers/gpu/drm/i915/gt/selftest_workarounds.c:492:17: error: 'rsvd' may be > used uninitialized in this function [-Werror=maybe-uninitialized] > > I could not figure out a good way to do this in a way that gcc > understands better, so initialize the variable to zero, as last > resort. Does it look neater if we initialise it as a local? No. Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/atomic-helper: reset vblank on crtc reset
On Wed, May 27, 2020 at 11:48 AM Daniel Vetter wrote: > > Only when vblanks are supported ofc. > > Some drivers do this already, but most unfortunately missed it. This > opens up bugs after driver load, before the crtc is enabled for the > first time. syzbot spotted this when loading vkms as a secondary > output. Given how many drivers are buggy it's best to solve this once > and for all in shared helper code. > > Aside from moving the few existing calls to drm_crtc_vblank_reset into > helpers (i915 doesn't use helpers, so keeps its own) I think the > regression risk is minimal: atomic helpers already rely on drivers > calling drm_crtc_vblank_on/off correctly in their hooks when they > support vblanks. And driver that's failing to handle vblanks after > this is missing those calls already, and vblanks could only work by > accident when enabling a CRTC for the first time right after boot. > > Big thanks to Tetsuo for helping track down what's going wrong here. > > There's only a few drivers which already had the necessary call and > needed some updating: > - komeda, atmel and tidss also needed to be changed to call > __drm_atomic_helper_crtc_reset() intead of open coding it > - tegra and msm even had it in the same place already, just code > motion, and malidp already uses __drm_atomic_helper_crtc_reset(). > > Only call left is in i915, which doesn't use drm_mode_config_reset, > but has its own fastboot infrastructure. So that's the only case where > we actually want this in the driver still. > > I've also reviewed all other drivers which set up vblank support with > drm_vblank_init. After the previous patch fixing mxsfb all atomic > drivers do call drm_crtc_vblank_on/off as they should, the remaining > drivers are either legacy kms or legacy dri1 drivers, so not affected > by this change to atomic helpers. > > Link: > https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb > Reported-by: Tetsuo Handa > Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com > Cc: Tetsuo Handa > Cc: "James (Qian) Wang" > Cc: Liviu Dudau > Cc: Mihail Atanassov > Cc: Brian Starkey > Cc: Sam Ravnborg > Cc: Boris Brezillon > Cc: Nicolas Ferre > Cc: Alexandre Belloni > Cc: Ludovic Desroches > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > Cc: Thierry Reding > Cc: Jonathan Hunter > Cc: Jyri Sarha > Cc: Tomi Valkeinen > Cc: Rob Clark > Cc: Sean Paul > Cc: Brian Masney > Cc: Emil Velikov > Cc: zhengbin > Cc: Thomas Gleixner > Cc: linux-te...@vger.kernel.org > Signed-off-by: Daniel Vetter For the atmel part from irc because Sam's isp refused to send out a mail with this many recipients: Reviewed-by: Sam Ravnborg Cheers, Daniel > --- > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 7 ++- > drivers/gpu/drm/arm/malidp_drv.c | 1 - > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 7 ++- > drivers/gpu/drm/drm_atomic_state_helper.c| 4 > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c| 2 -- > drivers/gpu/drm/tegra/dc.c | 1 - > drivers/gpu/drm/tidss/tidss_crtc.c | 3 +-- > drivers/gpu/drm/tidss/tidss_kms.c| 4 > 8 files changed, 9 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > index 56bd938961ee..f33418d6e1a0 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > @@ -492,10 +492,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc) > crtc->state = NULL; > > state = kzalloc(sizeof(*state), GFP_KERNEL); > - if (state) { > - crtc->state = >base; > - crtc->state->crtc = crtc; > - } > + if (state) > + __drm_atomic_helper_crtc_reset(crtc, >base); > } > > static struct drm_crtc_state * > @@ -616,7 +614,6 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms, > return err; > > drm_crtc_helper_add(crtc, _crtc_helper_funcs); > - drm_crtc_vblank_reset(crtc); > > crtc->port = kcrtc->master->of_output_port; > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > b/drivers/gpu/drm/arm/malidp_drv.c > index c2507b7d8512..02904392e370 100644 > --- a/drivers/gpu/drm/arm/malidp_drv.c > +++ b/drivers/gpu/drm/arm/malidp_drv.c > @@ -870,7 +870,6 @@ static int malidp_bind(struct device *dev) > drm->irq_enabled = true; > > ret = drm_vblank_init(drm, drm->mode_config.num_crtc); > - drm_crtc_vblank_reset(>crtc); > if (ret < 0) { > DRM_ERROR("failed to initialise vblank\n"); > goto vblank_fail; > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c > index 10985134ce0b..ce246b96330b 100644 > ---
Re: [PATCH 1/3] drm/i915/pmu: avoid an maybe-uninitialized warning
Quoting Arnd Bergmann (2020-05-27 15:05:08) > Conditional spinlocks make it hard for gcc and for lockdep to > follow the code flow. This one causes a warning with at least > gcc-9 and higher: > > In file included from include/linux/irq.h:14, > from drivers/gpu/drm/i915/i915_pmu.c:7: > drivers/gpu/drm/i915/i915_pmu.c: In function 'i915_sample': > include/linux/spinlock.h:289:3: error: 'flags' may be used uninitialized in > this function [-Werror=maybe-uninitialized] > 289 | _raw_spin_unlock_irqrestore(lock, flags); \ > | ^~~ > drivers/gpu/drm/i915/i915_pmu.c:288:17: note: 'flags' was declared here > 288 | unsigned long flags; > | ^ > > Split out the part between the locks into a separate function > for readability and to let the compiler figure out what the > logic actually is. > > Fixes: d79e1bd676f0 ("drm/i915/pmu: Only use exclusive mmio access for gen7") > Signed-off-by: Arnd Bergmann > --- > I have no idea why I see three separate issues like this pop up in i915, > there are not a lot of them elsewhere. gcc v8: add/remove: 1/0 grow/shrink: 0/1 up/down: 99/-164 (-65) Function old new delta engine_sample - 99 +99 i915_sample 871 707-164 Which is compelling. Is gcc 9 simliar? Given the above reduction, I find it hard to argue with. Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
On Wed, May 27, 2020 at 1:47 AM Sharat Masetty wrote: > > + more folks > > On 5/18/2020 9:55 PM, Rob Clark wrote: > > On Mon, May 18, 2020 at 7:23 AM Jordan Crouse > > wrote: > >> On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote: > >>> This patches replaces the previously used static DDR vote and uses > >>> dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling > >>> GPU frequency. > >>> > >>> Signed-off-by: Sharat Masetty > >>> --- > >>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +- > >>> 1 file changed, 1 insertion(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>> index 2d8124b..79433d3 100644 > >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >>> @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct > >>> dev_pm_opp *opp) > >>> > >>>gmu->freq = gmu->gpu_freqs[perf_index]; > >>> > >>> - /* > >>> - * Eventually we will want to scale the path vote with the > >>> frequency but > >>> - * for now leave it at max so that the performance is nominal. > >>> - */ > >>> - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > >>> + dev_pm_opp_set_bw(>pdev->dev, opp); > >>> } > >> This adds an implicit requirement that all targets need bandwidth settings > >> defined in the OPP or they won't get a bus vote at all. I would prefer that > >> there be an default escape valve but if not you'll need to add > >> bandwidth values for the sdm845 OPP that target doesn't regress. > >> > > it looks like we could maybe do something like: > > > >ret = dev_pm_opp_set_bw(...); > >if (ret) { > >dev_warn_once(dev, "no bandwidth settings"); > >icc_set_bw(...); > >} > > > > ? > > > > BR, > > -R > > There is a bit of an issue here - Looks like its not possible to two icc > handles to the same path. Its causing double enumeration of the paths > in the icc core and messing up path votes. With [1] Since opp/core > already gets a handle to the icc path as part of table add, drm/msm > could do either > > a) Conditionally enumerate gpu->icc_path handle only when pm/opp core > has not got the icc path handle. I could use something like [2] to > determine if should initialize gpu->icc_path* > > b) Add peak-opp-configs in 845 dt and mandate all future versions to use > this bindings. With this, I can remove gpu->icc_path from msm/drm > completely and only rely on opp/core for bw voting. The main thing is that we want to make sure newer dtb always works on an older kernel without regression.. but, hmm.. I guess the interconnects/interconnects-names properties haven't landed yet in sdm845.dtsi? Maybe that lets us go with the simpler approach (b). Looks like we haven't wired up interconnect for 8916 or 8996 either, so probably we can just mandate this for all of them? If we have landed the interconnect dts hookup for gpu somewhere that I'm overlooking, I guess we would have to go with (a) and keep the existing interconnects/interconnects-names properties. BR, -R > [1] - https://lore.kernel.org/patchwork/cover/1240687/ > > [2] - https://patchwork.kernel.org/patch/11527573/ > > Let me know your thoughts > > Sharat > > > > >> Jordan > >> > >>> unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) > >>> -- > >>> 2.7.4 > >>> > >> -- > >> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > >> a Linux Foundation Collaborative Project > >> ___ > >> Freedreno mailing list > >> freedr...@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/freedreno ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #9 from Maurice Gale (mauricega...@gmail.com) --- Created attachment 289349 --> https://bugzilla.kernel.org/attachment.cgi?id=289349=edit Log for 5.7 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #8 from Maurice Gale (mauricega...@gmail.com) --- Created attachment 289347 --> https://bugzilla.kernel.org/attachment.cgi?id=289347=edit Log for 5.6 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot
https://bugzilla.kernel.org/show_bug.cgi?id=207901 --- Comment #7 from Maurice Gale (mauricega...@gmail.com) --- Interestingly, after installing both 5.6 and 5.7-rc, I get no displays at all when connected to my Nvidia p2000. However, when I use integrated graphics, I get both of the displays. It seems as though things have gotten worse. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4.19.x] make 'user_access_begin()' do 'access_ok()'
On Wed, May 13, 2020 at 05:08:19PM +, Ashwin H wrote: > > Ok, but what does that mean for us? > > > > You need to say why you are sending a patch, otherwise we will guess wrong. > > In drivers/gpu/drm/i915/i915_gem_execbuffer.c, ioctl functions does > user_access_begin() without doing access_ok(Checks if a user space pointer is > valid) first. > A local attacker can craft a malicious ioctl function call to overwrite > arbitrary kernel memory, resulting in a Denial of Service or privilege > escalation (CVE-2018-20669) > > This patch makes sure that user_access_begin always does access_ok. > user_access_begin has been modified to do access_ok internally. I had this in the tree, but it broke the build on alpha, sh, and maybe a few others :( See: https://lore.kernel.org/r/20200527140225.ga214...@roeck-us.net for the details. Can you dig out all of the needed follow-on patches as well, and send them all as a patch series for 4.19.y so that I can queue them all up at once? thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: device hot-unplug for userspace
On 5/27/20 10:39 AM, Daniel Vetter wrote: On Wed, May 27, 2020 at 3:51 PM Andrey Grodzovsky wrote: On 5/27/20 2:44 AM, Pekka Paalanen wrote: On Tue, 26 May 2020 10:30:20 -0400 Andrey Grodzovsky wrote: On 5/19/20 6:06 AM, Pekka Paalanen wrote: From: Pekka Paalanen Set up the expectations on how hot-unplugging a DRM device should look like to userspace. Written by Daniel Vetter's request and largely based on his comments in IRC and from https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-May%2F265484.htmldata=02%7C01%7CAndrey.Grodzovsky%40amd.com%7C3c671803b2ba41b2ceac08d8024bcc9a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637261871869742519sdata=ZnhylRubOM0%2BjoreSSYMqVDzZuUdybEsoyBVcTKgxWE%3Dreserved=0 . Signed-off-by: Pekka Paalanen Cc: Daniel Vetter Cc: Andrey Grodzovsky Cc: Dave Airlie Cc: Sean Paul --- Disclaimer: I am a userspace developer writing for other userspace developers. I took some liberties in defining what should happen without knowing what is actually possible or what existing drivers already implement. --- Documentation/gpu/drm-uapi.rst | 75 ++ 1 file changed, 75 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 56fec6ed1ad8..80db4abd2cbd 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -1,3 +1,5 @@ +.. Copyright 2020 DisplayLink (UK) Ltd. + === Userland interfaces === @@ -162,6 +164,79 @@ other hand, a driver requires shared state between clients which is visible to user-space and accessible beyond open-file boundaries, they cannot support render nodes. +Device Hot-Unplug += + +.. note:: + The following is the plan. Implementation is not there yet + (2020 May 13). + +Graphics devices (display and/or render) may be connected via USB (e.g. +display adapters or docking stations) or Thunderbolt (e.g. eGPU). An end +user is able to hot-unplug this kind of devices while they are being +used, and expects that the very least the machine does not crash. Any +damage from hot-unplugging a DRM device needs to be limited as much as +possible and userspace must be given the chance to handle it if it wants +to. Ideally, unplugging a DRM device still lets a desktop to continue +running, but that is going to need explicit support throughout the whole +graphics stack: from kernel and userspace drivers, through display +servers, via window system protocols, and in applications and libraries. + +Other scenarios that should lead to the same are: unrecoverable GPU +crash, PCI device disappearing off the bus, or forced unbind of a driver +from the physical device. + +In other words, from userspace perspective everything needs to keep on +working more or less, until userspace stops using the disappeared DRM +device and closes it completely. Userspace will learn of the device +disappearance from the device removed uevent or in some cases specific +ioctls returning EIO. + +This goal raises at least the following requirements for the kernel and +drivers: + +- The kernel must not hang, crash or oops, no matter what userspace was + in the middle of doing when the device disappeared. + +- All GPU jobs that can no longer run must have their fences + force-signalled to avoid inflicting hangs to userspace. + +- KMS connectors must change their status to disconnected. + +- Legacy modesets and pageflips fake success. + +- Atomic commits, both real and TEST_ONLY, fake success. + +- Pending non-blocking KMS operations deliver the DRM events userspace + is expecting. + +- If underlying memory disappears, the mmaps are replaced with harmless + zero pages where access does not raise SIGBUS. Reads return zeros, + writes are ignored. Regarding this paragraph - what about exiting mappings ? In the first patchset we would actively invalidate all the existing CPU mappings to device memory and i think we still should do it otherwise we will see random crashes in applications as was before. I guess it's because TLBs and page tables are not updated to reflect the fact the device is gone. Hi, I was talking about existing mappings. What I forgot to specify is how new mmap() calls after the device disappearance should work (the end result should be the same still, not failure). I'll clarify this in the next revision. Thanks, pq I see, that ok. Next related question is more for Daniel/Christian - about the implementation of this paragraph, I was thinking about something like checking for device disconnect in ttm_bo_vm_fault_reserved and if so remap the entire VA range for the VMA where the fault address belongs to the global zero page (i.e. (remap_pfn_range(vma, vma->vm_start, page_to_pfn(ZERO_PAGE(vma->vm_start), vma->vm_end - vma->vm_start, vma->vm_page_prot)). Question is, when the doc says 'writes are ignored' does it mean i should use
Re: [PATCH] drm: pl111: add CONFIG_VEXPRESS_CONFIG dependency
Hi Arnd. On Wed, May 27, 2020 at 03:31:42PM +0200, Arnd Bergmann wrote: > The vexpress_config code fails to link in some configurations: > > drivers/gpu/drm/pl111/pl111_versatile.o: in function `pl111_versatile_init': > (.text+0x1f0): undefined reference to `devm_regmap_init_vexpress_config' > > Add a dependency that links to this only if the dependency is there, > and prevent the configuration where the drm driver is built-in but > the config is a loadable module. > > Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile init") > Signed-off-by: Arnd Bergmann Could this be another way to fix it: diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c index 64f01a4e6767..1c38d3bd2e84 100644 --- a/drivers/gpu/drm/pl111/pl111_versatile.c +++ b/drivers/gpu/drm/pl111/pl111_versatile.c @@ -379,7 +379,7 @@ static int pl111_vexpress_clcd_init(struct device *dev, struct device_node *np, u32 val; int ret; - if (!IS_ENABLED(CONFIG_VEXPRESS_CONFIG)) + if (!IS_REACHABLE(CONFIG_VEXPRESS_CONFIG)) return -ENODEV; /* Then we no longer have the whole driver depending on the value of VEXPRESS_CONFIG. Not that I like IS_REACHABLE() but we already had IS_ENABLED() to cover up here, and that was not enough. With your patch would we then need the IS_ENABLED() check? Sam > --- > drivers/gpu/drm/pl111/Kconfig | 1 + > drivers/gpu/drm/pl111/pl111_versatile.c | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig > index 80f6748055e3..33a005816fdd 100644 > --- a/drivers/gpu/drm/pl111/Kconfig > +++ b/drivers/gpu/drm/pl111/Kconfig > @@ -3,6 +3,7 @@ config DRM_PL111 > tristate "DRM Support for PL111 CLCD Controller" > depends on DRM > depends on ARM || ARM64 || COMPILE_TEST > + depends on VEXPRESS_CONFIG || !VEXPRESS_CONFIG > depends on COMMON_CLK > select DRM_KMS_HELPER > select DRM_KMS_CMA_HELPER > diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c > b/drivers/gpu/drm/pl111/pl111_versatile.c > index 64f01a4e6767..451d74205108 100644 > --- a/drivers/gpu/drm/pl111/pl111_versatile.c > +++ b/drivers/gpu/drm/pl111/pl111_versatile.c > @@ -476,7 +476,8 @@ int pl111_versatile_init(struct device *dev, struct > pl111_drm_dev_private *priv) > versatile_clcd_type = (enum versatile_clcd)clcd_id->data; > > /* Versatile Express special handling */ > - if (versatile_clcd_type == VEXPRESS_CLCD_V2M) { > + if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && > + versatile_clcd_type == VEXPRESS_CLCD_V2M) { > int ret = pl111_vexpress_clcd_init(dev, np, priv); > of_node_put(np); > if (ret) > -- > 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: device hot-unplug for userspace
On Wed, May 27, 2020 at 3:51 PM Andrey Grodzovsky wrote: > > > On 5/27/20 2:44 AM, Pekka Paalanen wrote: > > On Tue, 26 May 2020 10:30:20 -0400 > > Andrey Grodzovsky wrote: > > > >> On 5/19/20 6:06 AM, Pekka Paalanen wrote: > >>> From: Pekka Paalanen > >>> > >>> Set up the expectations on how hot-unplugging a DRM device should look > >>> like to > >>> userspace. > >>> > >>> Written by Daniel Vetter's request and largely based on his comments in > >>> IRC and > >>> from > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-May%2F265484.htmldata=02%7C01%7Candrey.grodzovsky%40amd.com%7Ce8e13dc4c85648e5fcd408d7fbdc5f2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637254796242596783sdata=bA%2FAy3VGvzNqmV1kGigNROSZQEws2E98JibDxvEICNs%3Dreserved=0 > >>> . > >>> > >>> Signed-off-by: Pekka Paalanen > >>> Cc: Daniel Vetter > >>> Cc: Andrey Grodzovsky > >>> Cc: Dave Airlie > >>> Cc: Sean Paul > >>> > >>> --- > >>> > >>> Disclaimer: I am a userspace developer writing for other userspace > >>> developers. > >>> I took some liberties in defining what should happen without knowing what > >>> is > >>> actually possible or what existing drivers already implement. > >>> --- > >>>Documentation/gpu/drm-uapi.rst | 75 ++ > >>>1 file changed, 75 insertions(+) > >>> > >>> diff --git a/Documentation/gpu/drm-uapi.rst > >>> b/Documentation/gpu/drm-uapi.rst > >>> index 56fec6ed1ad8..80db4abd2cbd 100644 > >>> --- a/Documentation/gpu/drm-uapi.rst > >>> +++ b/Documentation/gpu/drm-uapi.rst > >>> @@ -1,3 +1,5 @@ > >>> +.. Copyright 2020 DisplayLink (UK) Ltd. > >>> + > >>>=== > >>>Userland interfaces > >>>=== > >>> @@ -162,6 +164,79 @@ other hand, a driver requires shared state between > >>> clients which is > >>>visible to user-space and accessible beyond open-file boundaries, they > >>>cannot support render nodes. > >>> > >>> +Device Hot-Unplug > >>> += > >>> + > >>> +.. note:: > >>> + The following is the plan. Implementation is not there yet > >>> + (2020 May 13). > >>> + > >>> +Graphics devices (display and/or render) may be connected via USB (e.g. > >>> +display adapters or docking stations) or Thunderbolt (e.g. eGPU). An end > >>> +user is able to hot-unplug this kind of devices while they are being > >>> +used, and expects that the very least the machine does not crash. Any > >>> +damage from hot-unplugging a DRM device needs to be limited as much as > >>> +possible and userspace must be given the chance to handle it if it wants > >>> +to. Ideally, unplugging a DRM device still lets a desktop to continue > >>> +running, but that is going to need explicit support throughout the whole > >>> +graphics stack: from kernel and userspace drivers, through display > >>> +servers, via window system protocols, and in applications and libraries. > >>> + > >>> +Other scenarios that should lead to the same are: unrecoverable GPU > >>> +crash, PCI device disappearing off the bus, or forced unbind of a driver > >>> +from the physical device. > >>> + > >>> +In other words, from userspace perspective everything needs to keep on > >>> +working more or less, until userspace stops using the disappeared DRM > >>> +device and closes it completely. Userspace will learn of the device > >>> +disappearance from the device removed uevent or in some cases specific > >>> +ioctls returning EIO. > >>> + > >>> +This goal raises at least the following requirements for the kernel and > >>> +drivers: > >>> + > >>> +- The kernel must not hang, crash or oops, no matter what userspace was > >>> + in the middle of doing when the device disappeared. > >>> + > >>> +- All GPU jobs that can no longer run must have their fences > >>> + force-signalled to avoid inflicting hangs to userspace. > >>> + > >>> +- KMS connectors must change their status to disconnected. > >>> + > >>> +- Legacy modesets and pageflips fake success. > >>> + > >>> +- Atomic commits, both real and TEST_ONLY, fake success. > >>> + > >>> +- Pending non-blocking KMS operations deliver the DRM events userspace > >>> + is expecting. > >>> + > >>> +- If underlying memory disappears, the mmaps are replaced with harmless > >>> + zero pages where access does not raise SIGBUS. Reads return zeros, > >>> + writes are ignored. > >> > >> Regarding this paragraph - what about exiting mappings ? In the first > >> patchset we would actively invalidate all the existing CPU mappings to > >> device memory and i think we still should do it otherwise we will see > >> random crashes in applications as was before. I guess it's because TLBs > >> and page tables are not updated to reflect the fact the device is gone. > > Hi, > > > > I was talking about existing mappings. What I forgot to specify is how > > new mmap() calls after the device disappearance should work (the end > > result should be the same still, not failure). > >
Re: [Nouveau] [PATCH] nouveau: add fbdev dependency
On Wed, May 27, 2020 at 4:05 PM Ilia Mirkin wrote: > > Isn't this already fixed by > > https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403 Ok, I see that fixes the link error, but I when I created my fix, that did not seem like the correct solution because it reverts part of the original patch without reverting the rest of it. Unfortunately there was no changelog text in the first patch to explain why this is safe. Could you double-check if the behavior is still correct after the two patches? Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic-helper: reset vblank on crtc reset
On Wed, May 27, 2020 at 2:08 PM Liviu Dudau wrote: > > On Wed, May 27, 2020 at 01:07:05PM +0200, Daniel Vetter wrote: > > On Wed, May 27, 2020 at 12:57 PM Liviu Dudau wrote: > > > > > > Hi Daniel, > > > > > > On Wed, May 27, 2020 at 11:53:32AM +0200, Daniel Vetter wrote: > > > > Only when vblanks are supported ofc. > > > > > > > > Some drivers do this already, but most unfortunately missed it. This > > > > opens up bugs after driver load, before the crtc is enabled for the > > > > first time. syzbot spotted this when loading vkms as a secondary > > > > output. Given how many drivers are buggy it's best to solve this once > > > > and for all in shared helper code. > > > > > > > > Aside from moving the few existing calls to drm_crtc_vblank_reset into > > > > helpers (i915 doesn't use helpers, so keeps its own) I think the > > > > regression risk is minimal: atomic helpers already rely on drivers > > > > calling drm_crtc_vblank_on/off correctly in their hooks when they > > > > support vblanks. And driver that's failing to handle vblanks after > > > > this is missing those calls already, and vblanks could only work by > > > > accident when enabling a CRTC for the first time right after boot. > > > > > > > > Big thanks to Tetsuo for helping track down what's going wrong here. > > > > > > > > There's only a few drivers which already had the necessary call and > > > > needed some updating: > > > > - komeda, atmel and tidss also needed to be changed to call > > > > __drm_atomic_helper_crtc_reset() intead of open coding it > > > > - tegra and msm even had it in the same place already, just code > > > > motion, and malidp already uses __drm_atomic_helper_crtc_reset(). > > > > > > > > Only call left is in i915, which doesn't use drm_mode_config_reset, > > > > but has its own fastboot infrastructure. So that's the only case where > > > > we actually want this in the driver still. > > > > > > > > I've also reviewed all other drivers which set up vblank support with > > > > drm_vblank_init. After the previous patch fixing mxsfb all atomic > > > > drivers do call drm_crtc_vblank_on/off as they should, the remaining > > > > drivers are either legacy kms or legacy dri1 drivers, so not affected > > > > by this change to atomic helpers. > > > > > > > > v2: Use the drm_dev_has_vblank() helper. > > > > > > > > Link: > > > > https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb > > > > Reported-by: Tetsuo Handa > > > > Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com > > > > Cc: Tetsuo Handa > > > > Cc: "James (Qian) Wang" > > > > Cc: Liviu Dudau > > > > Cc: Mihail Atanassov > > > > Cc: Brian Starkey > > > > Cc: Sam Ravnborg > > > > Cc: Boris Brezillon > > > > Cc: Nicolas Ferre > > > > Cc: Alexandre Belloni > > > > Cc: Ludovic Desroches > > > > Cc: Maarten Lankhorst > > > > Cc: Maxime Ripard > > > > Cc: Thomas Zimmermann > > > > Cc: David Airlie > > > > Cc: Daniel Vetter > > > > Cc: Thierry Reding > > > > Cc: Jonathan Hunter > > > > Cc: Jyri Sarha > > > > Cc: Tomi Valkeinen > > > > Cc: Rob Clark > > > > Cc: Sean Paul > > > > Cc: Brian Masney > > > > Cc: Emil Velikov > > > > Cc: zhengbin > > > > Cc: Thomas Gleixner > > > > Cc: linux-te...@vger.kernel.org > > > > Signed-off-by: Daniel Vetter > > > > --- > > > > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 7 ++- > > > > drivers/gpu/drm/arm/malidp_drv.c | 1 - > > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 7 ++- > > > > drivers/gpu/drm/drm_atomic_state_helper.c| 4 > > > > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c| 2 -- > > > > drivers/gpu/drm/tegra/dc.c | 1 - > > > > drivers/gpu/drm/tidss/tidss_crtc.c | 3 +-- > > > > drivers/gpu/drm/tidss/tidss_kms.c| 4 > > > > 8 files changed, 9 insertions(+), 20 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > > > b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > > > index 56bd938961ee..f33418d6e1a0 100644 > > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > > > @@ -492,10 +492,8 @@ static void komeda_crtc_reset(struct drm_crtc > > > > *crtc) > > > > crtc->state = NULL; > > > > > > > > state = kzalloc(sizeof(*state), GFP_KERNEL); > > > > - if (state) { > > > > - crtc->state = >base; > > > > - crtc->state->crtc = crtc; > > > > - } > > > > + if (state) > > > > + __drm_atomic_helper_crtc_reset(crtc, >base); > > > > } > > > > > > > > static struct drm_crtc_state * > > > > @@ -616,7 +614,6 @@ static int komeda_crtc_add(struct komeda_kms_dev > > > > *kms, > > > > return err; > > > > > > > > drm_crtc_helper_add(crtc, _crtc_helper_funcs); > > > > - drm_crtc_vblank_reset(crtc); > > > > > > > > crtc->port =
[PATCH 2/3] drm/i915: work around false-positive maybe-uninitialized warning
gcc-9 gets confused by the code flow in check_dirty_whitelist: drivers/gpu/drm/i915/gt/selftest_workarounds.c: In function 'check_dirty_whitelist': drivers/gpu/drm/i915/gt/selftest_workarounds.c:492:17: error: 'rsvd' may be used uninitialized in this function [-Werror=maybe-uninitialized] I could not figure out a good way to do this in a way that gcc understands better, so initialize the variable to zero, as last resort. Fixes: aee20aaed887 ("drm/i915: Implement read-only support in whitelist selftest") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/i915/gt/selftest_workarounds.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c index 5ed323254ee1..32785463ec9e 100644 --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c @@ -623,6 +623,8 @@ static int check_dirty_whitelist(struct intel_context *ce) err = -EINVAL; goto out_unpin; } + } else { + rsvd = 0; } expect = results[0]; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] drm/i915/selftests: avoid bogus maybe-uninitialized warning
gcc has a problem following values through IS_ERR/PTR_ERR macros, leading to a false-positive warning in allmodconfig, with any compiler version: In file included from drivers/gpu/drm/i915/gt/intel_lrc.c:5892: drivers/gpu/drm/i915/gt/selftest_lrc.c: In function 'create_gpr_client.isra.0': drivers/gpu/drm/i915/gt/selftest_lrc.c:2902:23: error: 'rq' may be used uninitialized in this function [-Werror=maybe-uninitialized] This one is hard to avoid without impairing readability or adding a bugus NULL pointer. Fixes: c92724de6db1 ("drm/i915/selftests: Try to detect rollback during batchbuffer preemption") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/i915/gt/selftest_lrc.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c index 824f99c4cc7c..933c3f5adf0a 100644 --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c @@ -2908,23 +2908,25 @@ create_gpr_client(struct intel_engine_cs *engine, vma = i915_vma_instance(global->obj, ce->vm, NULL); if (IS_ERR(vma)) { - err = PTR_ERR(vma); + rq = ERR_CAST(vma); goto out_ce; } err = i915_vma_pin(vma, 0, 0, PIN_USER); - if (err) + if (err) { + rq = ERR_PTR(err); goto out_ce; + } batch = create_gpr_user(engine, vma, offset); if (IS_ERR(batch)) { - err = PTR_ERR(batch); + rq = ERR_CAST(batch); goto out_vma; } rq = intel_context_create_request(ce); if (IS_ERR(rq)) { - err = PTR_ERR(rq); + rq = ERR_CAST(rq); goto out_batch; } @@ -2946,17 +2948,20 @@ create_gpr_client(struct intel_engine_cs *engine, i915_vma_unlock(batch); i915_vma_unpin(batch); - if (!err) + if (!err) { i915_request_get(rq); - i915_request_add(rq); - + i915_request_add(rq); + } else { + i915_request_add(rq); + rq = ERR_PTR(err); + } out_batch: i915_vma_put(batch); out_vma: i915_vma_unpin(vma); out_ce: intel_context_put(ce); - return err ? ERR_PTR(err) : rq; + return rq; } static int preempt_user(struct intel_engine_cs *engine, -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/i915/pmu: avoid an maybe-uninitialized warning
Conditional spinlocks make it hard for gcc and for lockdep to follow the code flow. This one causes a warning with at least gcc-9 and higher: In file included from include/linux/irq.h:14, from drivers/gpu/drm/i915/i915_pmu.c:7: drivers/gpu/drm/i915/i915_pmu.c: In function 'i915_sample': include/linux/spinlock.h:289:3: error: 'flags' may be used uninitialized in this function [-Werror=maybe-uninitialized] 289 | _raw_spin_unlock_irqrestore(lock, flags); \ | ^~~ drivers/gpu/drm/i915/i915_pmu.c:288:17: note: 'flags' was declared here 288 | unsigned long flags; | ^ Split out the part between the locks into a separate function for readability and to let the compiler figure out what the logic actually is. Fixes: d79e1bd676f0 ("drm/i915/pmu: Only use exclusive mmio access for gen7") Signed-off-by: Arnd Bergmann --- I have no idea why I see three separate issues like this pop up in i915, there are not a lot of them elsewhere. drivers/gpu/drm/i915/i915_pmu.c | 84 - 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index e991a707bdb7..962ded9ce73f 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -269,12 +269,48 @@ static bool exclusive_mmio_access(const struct drm_i915_private *i915) return IS_GEN(i915, 7); } +static void engine_sample(struct intel_engine_cs *engine, unsigned int period_ns) +{ + struct intel_engine_pmu *pmu = >pmu; + bool busy; + u32 val; + + val = ENGINE_READ_FW(engine, RING_CTL); + if (val == 0) /* powerwell off => engine idle */ + return; + + if (val & RING_WAIT) + add_sample(>sample[I915_SAMPLE_WAIT], period_ns); + if (val & RING_WAIT_SEMAPHORE) + add_sample(>sample[I915_SAMPLE_SEMA], period_ns); + + /* No need to sample when busy stats are supported. */ + if (intel_engine_supports_stats(engine)) + return; + + /* +* While waiting on a semaphore or event, MI_MODE reports the +* ring as idle. However, previously using the seqno, and with +* execlists sampling, we account for the ring waiting as the +* engine being busy. Therefore, we record the sample as being +* busy if either waiting or !idle. +*/ + busy = val & (RING_WAIT_SEMAPHORE | RING_WAIT); + if (!busy) { + val = ENGINE_READ_FW(engine, RING_MI_MODE); + busy = !(val & MODE_IDLE); + } + if (busy) + add_sample(>sample[I915_SAMPLE_BUSY], period_ns); +} + static void engines_sample(struct intel_gt *gt, unsigned int period_ns) { struct drm_i915_private *i915 = gt->i915; struct intel_engine_cs *engine; enum intel_engine_id id; + unsigned long flags; if ((i915->pmu.enable & ENGINE_SAMPLE_MASK) == 0) return; @@ -283,53 +319,17 @@ engines_sample(struct intel_gt *gt, unsigned int period_ns) return; for_each_engine(engine, gt, id) { - struct intel_engine_pmu *pmu = >pmu; - spinlock_t *mmio_lock; - unsigned long flags; - bool busy; - u32 val; - if (!intel_engine_pm_get_if_awake(engine)) continue; - mmio_lock = NULL; - if (exclusive_mmio_access(i915)) - mmio_lock = >uncore->lock; - - if (unlikely(mmio_lock)) - spin_lock_irqsave(mmio_lock, flags); - - val = ENGINE_READ_FW(engine, RING_CTL); - if (val == 0) /* powerwell off => engine idle */ - goto skip; - - if (val & RING_WAIT) - add_sample(>sample[I915_SAMPLE_WAIT], period_ns); - if (val & RING_WAIT_SEMAPHORE) - add_sample(>sample[I915_SAMPLE_SEMA], period_ns); - - /* No need to sample when busy stats are supported. */ - if (intel_engine_supports_stats(engine)) - goto skip; - - /* -* While waiting on a semaphore or event, MI_MODE reports the -* ring as idle. However, previously using the seqno, and with -* execlists sampling, we account for the ring waiting as the -* engine being busy. Therefore, we record the sample as being -* busy if either waiting or !idle. -*/ - busy = val & (RING_WAIT_SEMAPHORE | RING_WAIT); - if (!busy) { - val = ENGINE_READ_FW(engine, RING_MI_MODE); - busy = !(val & MODE_IDLE); + if (exclusive_mmio_access(i915)) { +
Re: [Nouveau] [PATCH] nouveau: add fbdev dependency
Isn't this already fixed by https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403 On Wed, May 27, 2020 at 9:43 AM Arnd Bergmann wrote: > > Calling directly into the fbdev stack only works when the > fbdev layer is built into the kernel as well, or both are > loadable modules: > > drivers/gpu/drm/nouveau/nouveau_drm.o: in function `nouveau_drm_probe': > nouveau_drm.c:(.text+0x1f90): undefined reference to > `remove_conflicting_pci_framebuffers' > > The change seems to have been intentional, so add an explicit > dependency here but allow it to still be compiled if FBDEV > is completely disabled. > > Fixes: 2dd4d163cd9c ("drm/nouveau: remove open-coded version of > remove_conflicting_pci_framebuffers()") > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/drm/nouveau/Kconfig | 1 + > drivers/gpu/drm/nouveau/nouveau_drm.c | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig > index 980ed09bd7f6..8c640f003358 100644 > --- a/drivers/gpu/drm/nouveau/Kconfig > +++ b/drivers/gpu/drm/nouveau/Kconfig > @@ -18,6 +18,7 @@ config DRM_NOUVEAU > select THERMAL if ACPI && X86 > select ACPI_VIDEO if ACPI && X86 > select SND_HDA_COMPONENT if SND_HDA_CORE > + depends on FBDEV || !FBDEV > help > Choose this option for open-source NVIDIA support. > > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c > b/drivers/gpu/drm/nouveau/nouveau_drm.c > index eb10c80ed853..e8560444ab57 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -697,7 +697,8 @@ static int nouveau_drm_probe(struct pci_dev *pdev, > nvkm_device_del(); > > /* Remove conflicting drivers (vesafb, efifb etc). */ > - ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb"); > + if (IS_ENABLED(CONFIG_FBDEV)) > + ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb"); > if (ret) > return ret; > > -- > 2.26.2 > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/doc: device hot-unplug for userspace
On 5/27/20 2:44 AM, Pekka Paalanen wrote: On Tue, 26 May 2020 10:30:20 -0400 Andrey Grodzovsky wrote: On 5/19/20 6:06 AM, Pekka Paalanen wrote: From: Pekka Paalanen Set up the expectations on how hot-unplugging a DRM device should look like to userspace. Written by Daniel Vetter's request and largely based on his comments in IRC and from https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2020-May%2F265484.htmldata=02%7C01%7Candrey.grodzovsky%40amd.com%7Ce8e13dc4c85648e5fcd408d7fbdc5f2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637254796242596783sdata=bA%2FAy3VGvzNqmV1kGigNROSZQEws2E98JibDxvEICNs%3Dreserved=0 . Signed-off-by: Pekka Paalanen Cc: Daniel Vetter Cc: Andrey Grodzovsky Cc: Dave Airlie Cc: Sean Paul --- Disclaimer: I am a userspace developer writing for other userspace developers. I took some liberties in defining what should happen without knowing what is actually possible or what existing drivers already implement. --- Documentation/gpu/drm-uapi.rst | 75 ++ 1 file changed, 75 insertions(+) diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 56fec6ed1ad8..80db4abd2cbd 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -1,3 +1,5 @@ +.. Copyright 2020 DisplayLink (UK) Ltd. + === Userland interfaces === @@ -162,6 +164,79 @@ other hand, a driver requires shared state between clients which is visible to user-space and accessible beyond open-file boundaries, they cannot support render nodes. +Device Hot-Unplug += + +.. note:: + The following is the plan. Implementation is not there yet + (2020 May 13). + +Graphics devices (display and/or render) may be connected via USB (e.g. +display adapters or docking stations) or Thunderbolt (e.g. eGPU). An end +user is able to hot-unplug this kind of devices while they are being +used, and expects that the very least the machine does not crash. Any +damage from hot-unplugging a DRM device needs to be limited as much as +possible and userspace must be given the chance to handle it if it wants +to. Ideally, unplugging a DRM device still lets a desktop to continue +running, but that is going to need explicit support throughout the whole +graphics stack: from kernel and userspace drivers, through display +servers, via window system protocols, and in applications and libraries. + +Other scenarios that should lead to the same are: unrecoverable GPU +crash, PCI device disappearing off the bus, or forced unbind of a driver +from the physical device. + +In other words, from userspace perspective everything needs to keep on +working more or less, until userspace stops using the disappeared DRM +device and closes it completely. Userspace will learn of the device +disappearance from the device removed uevent or in some cases specific +ioctls returning EIO. + +This goal raises at least the following requirements for the kernel and +drivers: + +- The kernel must not hang, crash or oops, no matter what userspace was + in the middle of doing when the device disappeared. + +- All GPU jobs that can no longer run must have their fences + force-signalled to avoid inflicting hangs to userspace. + +- KMS connectors must change their status to disconnected. + +- Legacy modesets and pageflips fake success. + +- Atomic commits, both real and TEST_ONLY, fake success. + +- Pending non-blocking KMS operations deliver the DRM events userspace + is expecting. + +- If underlying memory disappears, the mmaps are replaced with harmless + zero pages where access does not raise SIGBUS. Reads return zeros, + writes are ignored. Regarding this paragraph - what about exiting mappings ? In the first patchset we would actively invalidate all the existing CPU mappings to device memory and i think we still should do it otherwise we will see random crashes in applications as was before. I guess it's because TLBs and page tables are not updated to reflect the fact the device is gone. Hi, I was talking about existing mappings. What I forgot to specify is how new mmap() calls after the device disappearance should work (the end result should be the same still, not failure). I'll clarify this in the next revision. Thanks, pq I see, that ok. Next related question is more for Daniel/Christian - about the implementation of this paragraph, I was thinking about something like checking for device disconnect in ttm_bo_vm_fault_reserved and if so remap the entire VA range for the VMA where the fault address belongs to the global zero page (i.e. (remap_pfn_range(vma, vma->vm_start, page_to_pfn(ZERO_PAGE(vma->vm_start), vma->vm_end - vma->vm_start, vma->vm_page_prot)). Question is, when the doc says 'writes are ignored' does it mean i should use copy on write for the vma->vm_page_prot and if so how i actually do it as i was not able to
[PATCH] nouveau: add fbdev dependency
Calling directly into the fbdev stack only works when the fbdev layer is built into the kernel as well, or both are loadable modules: drivers/gpu/drm/nouveau/nouveau_drm.o: in function `nouveau_drm_probe': nouveau_drm.c:(.text+0x1f90): undefined reference to `remove_conflicting_pci_framebuffers' The change seems to have been intentional, so add an explicit dependency here but allow it to still be compiled if FBDEV is completely disabled. Fixes: 2dd4d163cd9c ("drm/nouveau: remove open-coded version of remove_conflicting_pci_framebuffers()") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/nouveau/Kconfig | 1 + drivers/gpu/drm/nouveau/nouveau_drm.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index 980ed09bd7f6..8c640f003358 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -18,6 +18,7 @@ config DRM_NOUVEAU select THERMAL if ACPI && X86 select ACPI_VIDEO if ACPI && X86 select SND_HDA_COMPONENT if SND_HDA_CORE + depends on FBDEV || !FBDEV help Choose this option for open-source NVIDIA support. diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index eb10c80ed853..e8560444ab57 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -697,7 +697,8 @@ static int nouveau_drm_probe(struct pci_dev *pdev, nvkm_device_del(); /* Remove conflicting drivers (vesafb, efifb etc). */ - ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb"); + if (IS_ENABLED(CONFIG_FBDEV)) + ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb"); if (ret) return ret; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: pl111: add CONFIG_VEXPRESS_CONFIG dependency
On Wed, May 27, 2020 at 3:32 PM Arnd Bergmann wrote: > The vexpress_config code fails to link in some configurations: > > drivers/gpu/drm/pl111/pl111_versatile.o: in function `pl111_versatile_init': > (.text+0x1f0): undefined reference to `devm_regmap_init_vexpress_config' > > Add a dependency that links to this only if the dependency is there, > and prevent the configuration where the drm driver is built-in but > the config is a loadable module. > > Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile init") > Signed-off-by: Arnd Bergmann Reviewed-by: Linus Walleij Rob will you apply this directly to the drm tree? Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: pl111: add CONFIG_VEXPRESS_CONFIG dependency
The vexpress_config code fails to link in some configurations: drivers/gpu/drm/pl111/pl111_versatile.o: in function `pl111_versatile_init': (.text+0x1f0): undefined reference to `devm_regmap_init_vexpress_config' Add a dependency that links to this only if the dependency is there, and prevent the configuration where the drm driver is built-in but the config is a loadable module. Fixes: 826fc86b5903 ("drm: pl111: Move VExpress setup into versatile init") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/pl111/Kconfig | 1 + drivers/gpu/drm/pl111/pl111_versatile.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/pl111/Kconfig b/drivers/gpu/drm/pl111/Kconfig index 80f6748055e3..33a005816fdd 100644 --- a/drivers/gpu/drm/pl111/Kconfig +++ b/drivers/gpu/drm/pl111/Kconfig @@ -3,6 +3,7 @@ config DRM_PL111 tristate "DRM Support for PL111 CLCD Controller" depends on DRM depends on ARM || ARM64 || COMPILE_TEST + depends on VEXPRESS_CONFIG || !VEXPRESS_CONFIG depends on COMMON_CLK select DRM_KMS_HELPER select DRM_KMS_CMA_HELPER diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c index 64f01a4e6767..451d74205108 100644 --- a/drivers/gpu/drm/pl111/pl111_versatile.c +++ b/drivers/gpu/drm/pl111/pl111_versatile.c @@ -476,7 +476,8 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv) versatile_clcd_type = (enum versatile_clcd)clcd_id->data; /* Versatile Express special handling */ - if (versatile_clcd_type == VEXPRESS_CLCD_V2M) { + if (IS_ENABLED(CONFIG_VEXPRESS_CONFIG) && + versatile_clcd_type == VEXPRESS_CLCD_V2M) { int ret = pl111_vexpress_clcd_init(dev, np, priv); of_node_put(np); if (ret) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/edid: Iterate through all DispID ext blocks
From: Ville Syrjälä Apparently there are EDIDs in the wild with multiple DispID extension blocks. Iterate through them all. In one particular case the tile information is specicied in the second DispID ext block, and since the current parser only looks at the first DispID ext block we don't notice that we're dealing with a tiled display. While at it change a few functions to return void since we have no use for the errno. References: https://gitlab.freedesktop.org/drm/intel/-/issues/27 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 84 +- 1 file changed, 38 insertions(+), 46 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f2531d51dfa2..dcb23563d29d 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3248,6 +3248,7 @@ static u8 *drm_find_cea_extension(const struct edid *edid) int ext_index; /* Look for a top level CEA extension block */ + /* FIXME: make callers iterate through multiple CEA ext blocks? */ ext_index = 0; cea = drm_find_edid_extension(edid, CEA_EXT, _index); if (cea) @@ -3255,20 +3256,20 @@ static u8 *drm_find_cea_extension(const struct edid *edid) /* CEA blocks can also be found embedded in a DisplayID block */ ext_index = 0; - displayid = drm_find_displayid_extension(edid, , , -_index); - if (!displayid) - return NULL; + for (;;) { + displayid = drm_find_displayid_extension(edid, , , +_index); + if (!displayid) + return NULL; - idx += sizeof(struct displayid_hdr); - for_each_displayid_db(displayid, block, idx, length) { - if (block->tag == DATA_BLOCK_CTA) { - cea = (u8 *)block; - break; + idx += sizeof(struct displayid_hdr); + for_each_displayid_db(displayid, block, idx, length) { + if (block->tag == DATA_BLOCK_CTA) + return (u8 *)block; } } - return cea; + return NULL; } static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic) @@ -5205,19 +5206,22 @@ static int add_displayid_detailed_modes(struct drm_connector *connector, int num_modes = 0; int ext_index = 0; - displayid = drm_find_displayid_extension(edid, , , -_index); - if (!displayid) - return 0; - - idx += sizeof(struct displayid_hdr); - for_each_displayid_db(displayid, block, idx, length) { - switch (block->tag) { - case DATA_BLOCK_TYPE_1_DETAILED_TIMING: - num_modes += add_displayid_detailed_1_modes(connector, block); + for (;;) { + displayid = drm_find_displayid_extension(edid, , , +_index); + if (!displayid) break; + + idx += sizeof(struct displayid_hdr); + for_each_displayid_db(displayid, block, idx, length) { + switch (block->tag) { + case DATA_BLOCK_TYPE_1_DETAILED_TIMING: + num_modes += add_displayid_detailed_1_modes(connector, block); + break; + } } } + return num_modes; } @@ -5797,8 +5801,8 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, } EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode); -static int drm_parse_tiled_block(struct drm_connector *connector, -const struct displayid_block *block) +static void drm_parse_tiled_block(struct drm_connector *connector, + const struct displayid_block *block) { const struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block; u16 w, h; @@ -5836,7 +5840,7 @@ static int drm_parse_tiled_block(struct drm_connector *connector, tg = drm_mode_create_tile_group(connector->dev, tile->topology_id); } if (!tg) - return -ENOMEM; + return; if (connector->tile_group != tg) { /* if we haven't got a pointer, @@ -5848,14 +5852,12 @@ static int drm_parse_tiled_block(struct drm_connector *connector, } else /* if same tile group, then release the ref we just took. */ drm_mode_put_tile_group(connector->dev, tg); - return 0; } -static int drm_displayid_parse_tiled(struct drm_connector *connector, -const u8 *displayid, int length, int idx) +static void
[PATCH 3/3] drm/edid: Clean up some curly braces
From: Ville Syrjälä Drop some pointless curly braces, and add some across the else when the if has them too. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index dcb23563d29d..8a951e2bfb41 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5836,22 +5836,21 @@ static void drm_parse_tiled_block(struct drm_connector *connector, DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]); tg = drm_mode_get_tile_group(connector->dev, tile->topology_id); - if (!tg) { + if (!tg) tg = drm_mode_create_tile_group(connector->dev, tile->topology_id); - } if (!tg) return; if (connector->tile_group != tg) { /* if we haven't got a pointer, take the reference, drop ref to old tile group */ - if (connector->tile_group) { + if (connector->tile_group) drm_mode_put_tile_group(connector->dev, connector->tile_group); - } connector->tile_group = tg; - } else + } else { /* if same tile group, then release the ref we just took. */ drm_mode_put_tile_group(connector->dev, tg); + } } static void drm_displayid_parse_tiled(struct drm_connector *connector, -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/edid: Allow looking for ext blocks starting from a specified index
From: Ville Syrjälä Apparently EDIDs with multiple DispID ext blocks is a thing, so prepare for iterating through multiple ext blocks of the same type by passing the starting ext block index to drm_find_edid_extension(). Well also have drm_find_edid_extension() update the index to point to the next ext block on success. Thus we should be able to call drm_find_edid_extension() in loop. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index d8372d63851b..f2531d51dfa2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3188,7 +3188,8 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, /* * Search EDID for CEA extension block. */ -static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) +static u8 *drm_find_edid_extension(const struct edid *edid, + int ext_id, int *ext_index) { u8 *edid_ext = NULL; int i; @@ -3198,23 +3199,26 @@ static u8 *drm_find_edid_extension(const struct edid *edid, int ext_id) return NULL; /* Find CEA extension */ - for (i = 0; i < edid->extensions; i++) { + for (i = *ext_index; i < edid->extensions; i++) { edid_ext = (u8 *)edid + EDID_LENGTH * (i + 1); if (edid_ext[0] == ext_id) break; } - if (i == edid->extensions) + if (i >= edid->extensions) return NULL; + *ext_index = i + 1; + return edid_ext; } static u8 *drm_find_displayid_extension(const struct edid *edid, - int *length, int *idx) + int *length, int *idx, + int *ext_index) { - u8 *displayid = drm_find_edid_extension(edid, DISPLAYID_EXT); + u8 *displayid = drm_find_edid_extension(edid, DISPLAYID_EXT, ext_index); struct displayid_hdr *base; int ret; @@ -3241,14 +3245,18 @@ static u8 *drm_find_cea_extension(const struct edid *edid) struct displayid_block *block; u8 *cea; u8 *displayid; + int ext_index; /* Look for a top level CEA extension block */ - cea = drm_find_edid_extension(edid, CEA_EXT); + ext_index = 0; + cea = drm_find_edid_extension(edid, CEA_EXT, _index); if (cea) return cea; /* CEA blocks can also be found embedded in a DisplayID block */ - displayid = drm_find_displayid_extension(edid, , ); + ext_index = 0; + displayid = drm_find_displayid_extension(edid, , , +_index); if (!displayid) return NULL; @@ -5195,8 +5203,10 @@ static int add_displayid_detailed_modes(struct drm_connector *connector, int length, idx; struct displayid_block *block; int num_modes = 0; + int ext_index = 0; - displayid = drm_find_displayid_extension(edid, , ); + displayid = drm_find_displayid_extension(edid, , , +_index); if (!displayid) return 0; @@ -5870,11 +5880,13 @@ void drm_update_tile_info(struct drm_connector *connector, const struct edid *edid) { const void *displayid = NULL; + int ext_index = 0; int length, idx; int ret; connector->has_tile = false; - displayid = drm_find_displayid_extension(edid, , ); + displayid = drm_find_displayid_extension(edid, , , +_index); if (!displayid) { /* drop reference to any tile group we had */ goto out_drop_ref; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic-helper: reset vblank on crtc reset
On Wed, May 27, 2020 at 01:07:05PM +0200, Daniel Vetter wrote: > On Wed, May 27, 2020 at 12:57 PM Liviu Dudau wrote: > > > > Hi Daniel, > > > > On Wed, May 27, 2020 at 11:53:32AM +0200, Daniel Vetter wrote: > > > Only when vblanks are supported ofc. > > > > > > Some drivers do this already, but most unfortunately missed it. This > > > opens up bugs after driver load, before the crtc is enabled for the > > > first time. syzbot spotted this when loading vkms as a secondary > > > output. Given how many drivers are buggy it's best to solve this once > > > and for all in shared helper code. > > > > > > Aside from moving the few existing calls to drm_crtc_vblank_reset into > > > helpers (i915 doesn't use helpers, so keeps its own) I think the > > > regression risk is minimal: atomic helpers already rely on drivers > > > calling drm_crtc_vblank_on/off correctly in their hooks when they > > > support vblanks. And driver that's failing to handle vblanks after > > > this is missing those calls already, and vblanks could only work by > > > accident when enabling a CRTC for the first time right after boot. > > > > > > Big thanks to Tetsuo for helping track down what's going wrong here. > > > > > > There's only a few drivers which already had the necessary call and > > > needed some updating: > > > - komeda, atmel and tidss also needed to be changed to call > > > __drm_atomic_helper_crtc_reset() intead of open coding it > > > - tegra and msm even had it in the same place already, just code > > > motion, and malidp already uses __drm_atomic_helper_crtc_reset(). > > > > > > Only call left is in i915, which doesn't use drm_mode_config_reset, > > > but has its own fastboot infrastructure. So that's the only case where > > > we actually want this in the driver still. > > > > > > I've also reviewed all other drivers which set up vblank support with > > > drm_vblank_init. After the previous patch fixing mxsfb all atomic > > > drivers do call drm_crtc_vblank_on/off as they should, the remaining > > > drivers are either legacy kms or legacy dri1 drivers, so not affected > > > by this change to atomic helpers. > > > > > > v2: Use the drm_dev_has_vblank() helper. > > > > > > Link: > > > https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb > > > Reported-by: Tetsuo Handa > > > Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com > > > Cc: Tetsuo Handa > > > Cc: "James (Qian) Wang" > > > Cc: Liviu Dudau > > > Cc: Mihail Atanassov > > > Cc: Brian Starkey > > > Cc: Sam Ravnborg > > > Cc: Boris Brezillon > > > Cc: Nicolas Ferre > > > Cc: Alexandre Belloni > > > Cc: Ludovic Desroches > > > Cc: Maarten Lankhorst > > > Cc: Maxime Ripard > > > Cc: Thomas Zimmermann > > > Cc: David Airlie > > > Cc: Daniel Vetter > > > Cc: Thierry Reding > > > Cc: Jonathan Hunter > > > Cc: Jyri Sarha > > > Cc: Tomi Valkeinen > > > Cc: Rob Clark > > > Cc: Sean Paul > > > Cc: Brian Masney > > > Cc: Emil Velikov > > > Cc: zhengbin > > > Cc: Thomas Gleixner > > > Cc: linux-te...@vger.kernel.org > > > Signed-off-by: Daniel Vetter > > > --- > > > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 7 ++- > > > drivers/gpu/drm/arm/malidp_drv.c | 1 - > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 7 ++- > > > drivers/gpu/drm/drm_atomic_state_helper.c| 4 > > > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c| 2 -- > > > drivers/gpu/drm/tegra/dc.c | 1 - > > > drivers/gpu/drm/tidss/tidss_crtc.c | 3 +-- > > > drivers/gpu/drm/tidss/tidss_kms.c| 4 > > > 8 files changed, 9 insertions(+), 20 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > > b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > > index 56bd938961ee..f33418d6e1a0 100644 > > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > > @@ -492,10 +492,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc) > > > crtc->state = NULL; > > > > > > state = kzalloc(sizeof(*state), GFP_KERNEL); > > > - if (state) { > > > - crtc->state = >base; > > > - crtc->state->crtc = crtc; > > > - } > > > + if (state) > > > + __drm_atomic_helper_crtc_reset(crtc, >base); > > > } > > > > > > static struct drm_crtc_state * > > > @@ -616,7 +614,6 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms, > > > return err; > > > > > > drm_crtc_helper_add(crtc, _crtc_helper_funcs); > > > - drm_crtc_vblank_reset(crtc); > > > > > > crtc->port = kcrtc->master->of_output_port; > > > > > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > > b/drivers/gpu/drm/arm/malidp_drv.c > > > index c2507b7d8512..02904392e370 100644 > > > --- a/drivers/gpu/drm/arm/malidp_drv.c > > > +++ b/drivers/gpu/drm/arm/malidp_drv.c > > > @@ -870,7 +870,6 @@ static int
Re: [PATCH] drm: use drm_dev_has_vblank more
Am 27.05.20 um 13:11 schrieb Daniel Vetter: > For historical reasons it's called dev->num_crtcs, which is rather > confusing ever since kms was added. But now we have a nice helper, so > let's use it for better readability! > > Only code change is in atomic helpers: vblank support means that > dev->irq_enabled must be set too. Another one of these quirky things > ... But since it's implied we can simplify that check. > > Signed-off-by: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter Reviewed-by: Thomas Zimmermann > --- > drivers/gpu/drm/drm_atomic_helper.c | 2 +- > drivers/gpu/drm/drm_irq.c | 2 +- > drivers/gpu/drm/drm_vblank.c| 14 +++--- > 3 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 0a541368246e..bfcc7857a9a1 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -1097,7 +1097,7 @@ disable_outputs(struct drm_device *dev, struct > drm_atomic_state *old_state) > else if (funcs->dpms) > funcs->dpms(crtc, DRM_MODE_DPMS_OFF); > > - if (!(dev->irq_enabled && dev->num_crtcs)) > + if (!drm_dev_has_vblank(dev)) > continue; > > ret = drm_crtc_vblank_get(crtc); > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 588be45abd7a..09d6e9e2e075 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -181,7 +181,7 @@ int drm_irq_uninstall(struct drm_device *dev) >* vblank/irq handling. KMS drivers must ensure that vblanks are all >* disabled when uninstalling the irq handler. >*/ > - if (dev->num_crtcs) { > + if (drm_dev_has_vblank(dev)) { > spin_lock_irqsave(>vbl_lock, irqflags); > for (i = 0; i < dev->num_crtcs; i++) { > struct drm_vblank_crtc *vblank = >vblank[i]; > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index e278d6407f8e..162d9f7e692a 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -605,7 +605,7 @@ void drm_calc_timestamping_constants(struct drm_crtc > *crtc, > int linedur_ns = 0, framedur_ns = 0; > int dotclock = mode->crtc_clock; > > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return; > > if (WARN_ON(pipe >= dev->num_crtcs)) > @@ -1065,7 +1065,7 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc, > unsigned int pipe = drm_crtc_index(crtc); > ktime_t now; > > - if (dev->num_crtcs > 0) { > + if (drm_dev_has_vblank(dev)) { > seq = drm_vblank_count_and_time(dev, pipe, ); > } else { > seq = 0; > @@ -1137,7 +1137,7 @@ static int drm_vblank_get(struct drm_device *dev, > unsigned int pipe) > unsigned long irqflags; > int ret = 0; > > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return -EINVAL; > > if (WARN_ON(pipe >= dev->num_crtcs)) > @@ -1506,7 +1506,7 @@ static void drm_legacy_vblank_pre_modeset(struct > drm_device *dev, > struct drm_vblank_crtc *vblank = >vblank[pipe]; > > /* vblank is not initialized (IRQ not installed ?), or has been freed */ > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return; > > if (WARN_ON(pipe >= dev->num_crtcs)) > @@ -1533,7 +1533,7 @@ static void drm_legacy_vblank_post_modeset(struct > drm_device *dev, > unsigned long irqflags; > > /* vblank is not initialized (IRQ not installed ?), or has been freed */ > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return; > > if (WARN_ON(pipe >= dev->num_crtcs)) > @@ -1558,7 +1558,7 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device > *dev, void *data, > unsigned int pipe; > > /* If drm_vblank_init() hasn't been called yet, just no-op */ > - if (!dev->num_crtcs) > + if (!drm_dev_has_vblank(dev)) > return 0; > > /* KMS drivers handle this internally */ > @@ -1896,7 +1896,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned > int pipe) > unsigned long irqflags; > bool disable_irq, fence_cookie; > > - if (WARN_ON_ONCE(!dev->num_crtcs)) > + if (WARN_ON_ONCE(!drm_dev_has_vblank(dev))) > return false; > > if (WARN_ON(pipe >= dev->num_crtcs)) > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org
Re: [PATCH v6 8/9] spi: stm32: Add 'SPI_SIMPLEX_RX', 'SPI_3WIRE_RX' support for stm32f4
On Wed, May 27, 2020 at 06:45:53PM +0800, dillon min wrote: > sorry, forget to remove these two patch from this submits, will not > include it in later submits > which ack other's review result. Ah, OK - no problem. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: use drm_dev_has_vblank more
For historical reasons it's called dev->num_crtcs, which is rather confusing ever since kms was added. But now we have a nice helper, so let's use it for better readability! Only code change is in atomic helpers: vblank support means that dev->irq_enabled must be set too. Another one of these quirky things ... But since it's implied we can simplify that check. Signed-off-by: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter --- drivers/gpu/drm/drm_atomic_helper.c | 2 +- drivers/gpu/drm/drm_irq.c | 2 +- drivers/gpu/drm/drm_vblank.c| 14 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 0a541368246e..bfcc7857a9a1 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1097,7 +1097,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) else if (funcs->dpms) funcs->dpms(crtc, DRM_MODE_DPMS_OFF); - if (!(dev->irq_enabled && dev->num_crtcs)) + if (!drm_dev_has_vblank(dev)) continue; ret = drm_crtc_vblank_get(crtc); diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 588be45abd7a..09d6e9e2e075 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -181,7 +181,7 @@ int drm_irq_uninstall(struct drm_device *dev) * vblank/irq handling. KMS drivers must ensure that vblanks are all * disabled when uninstalling the irq handler. */ - if (dev->num_crtcs) { + if (drm_dev_has_vblank(dev)) { spin_lock_irqsave(>vbl_lock, irqflags); for (i = 0; i < dev->num_crtcs; i++) { struct drm_vblank_crtc *vblank = >vblank[i]; diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index e278d6407f8e..162d9f7e692a 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -605,7 +605,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc, int linedur_ns = 0, framedur_ns = 0; int dotclock = mode->crtc_clock; - if (!dev->num_crtcs) + if (!drm_dev_has_vblank(dev)) return; if (WARN_ON(pipe >= dev->num_crtcs)) @@ -1065,7 +1065,7 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc, unsigned int pipe = drm_crtc_index(crtc); ktime_t now; - if (dev->num_crtcs > 0) { + if (drm_dev_has_vblank(dev)) { seq = drm_vblank_count_and_time(dev, pipe, ); } else { seq = 0; @@ -1137,7 +1137,7 @@ static int drm_vblank_get(struct drm_device *dev, unsigned int pipe) unsigned long irqflags; int ret = 0; - if (!dev->num_crtcs) + if (!drm_dev_has_vblank(dev)) return -EINVAL; if (WARN_ON(pipe >= dev->num_crtcs)) @@ -1506,7 +1506,7 @@ static void drm_legacy_vblank_pre_modeset(struct drm_device *dev, struct drm_vblank_crtc *vblank = >vblank[pipe]; /* vblank is not initialized (IRQ not installed ?), or has been freed */ - if (!dev->num_crtcs) + if (!drm_dev_has_vblank(dev)) return; if (WARN_ON(pipe >= dev->num_crtcs)) @@ -1533,7 +1533,7 @@ static void drm_legacy_vblank_post_modeset(struct drm_device *dev, unsigned long irqflags; /* vblank is not initialized (IRQ not installed ?), or has been freed */ - if (!dev->num_crtcs) + if (!drm_dev_has_vblank(dev)) return; if (WARN_ON(pipe >= dev->num_crtcs)) @@ -1558,7 +1558,7 @@ int drm_legacy_modeset_ctl_ioctl(struct drm_device *dev, void *data, unsigned int pipe; /* If drm_vblank_init() hasn't been called yet, just no-op */ - if (!dev->num_crtcs) + if (!drm_dev_has_vblank(dev)) return 0; /* KMS drivers handle this internally */ @@ -1896,7 +1896,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) unsigned long irqflags; bool disable_irq, fence_cookie; - if (WARN_ON_ONCE(!dev->num_crtcs)) + if (WARN_ON_ONCE(!drm_dev_has_vblank(dev))) return false; if (WARN_ON(pipe >= dev->num_crtcs)) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic-helper: reset vblank on crtc reset
On Wed, May 27, 2020 at 12:57 PM Liviu Dudau wrote: > > Hi Daniel, > > On Wed, May 27, 2020 at 11:53:32AM +0200, Daniel Vetter wrote: > > Only when vblanks are supported ofc. > > > > Some drivers do this already, but most unfortunately missed it. This > > opens up bugs after driver load, before the crtc is enabled for the > > first time. syzbot spotted this when loading vkms as a secondary > > output. Given how many drivers are buggy it's best to solve this once > > and for all in shared helper code. > > > > Aside from moving the few existing calls to drm_crtc_vblank_reset into > > helpers (i915 doesn't use helpers, so keeps its own) I think the > > regression risk is minimal: atomic helpers already rely on drivers > > calling drm_crtc_vblank_on/off correctly in their hooks when they > > support vblanks. And driver that's failing to handle vblanks after > > this is missing those calls already, and vblanks could only work by > > accident when enabling a CRTC for the first time right after boot. > > > > Big thanks to Tetsuo for helping track down what's going wrong here. > > > > There's only a few drivers which already had the necessary call and > > needed some updating: > > - komeda, atmel and tidss also needed to be changed to call > > __drm_atomic_helper_crtc_reset() intead of open coding it > > - tegra and msm even had it in the same place already, just code > > motion, and malidp already uses __drm_atomic_helper_crtc_reset(). > > > > Only call left is in i915, which doesn't use drm_mode_config_reset, > > but has its own fastboot infrastructure. So that's the only case where > > we actually want this in the driver still. > > > > I've also reviewed all other drivers which set up vblank support with > > drm_vblank_init. After the previous patch fixing mxsfb all atomic > > drivers do call drm_crtc_vblank_on/off as they should, the remaining > > drivers are either legacy kms or legacy dri1 drivers, so not affected > > by this change to atomic helpers. > > > > v2: Use the drm_dev_has_vblank() helper. > > > > Link: > > https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb > > Reported-by: Tetsuo Handa > > Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com > > Cc: Tetsuo Handa > > Cc: "James (Qian) Wang" > > Cc: Liviu Dudau > > Cc: Mihail Atanassov > > Cc: Brian Starkey > > Cc: Sam Ravnborg > > Cc: Boris Brezillon > > Cc: Nicolas Ferre > > Cc: Alexandre Belloni > > Cc: Ludovic Desroches > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Thomas Zimmermann > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Thierry Reding > > Cc: Jonathan Hunter > > Cc: Jyri Sarha > > Cc: Tomi Valkeinen > > Cc: Rob Clark > > Cc: Sean Paul > > Cc: Brian Masney > > Cc: Emil Velikov > > Cc: zhengbin > > Cc: Thomas Gleixner > > Cc: linux-te...@vger.kernel.org > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 7 ++- > > drivers/gpu/drm/arm/malidp_drv.c | 1 - > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 7 ++- > > drivers/gpu/drm/drm_atomic_state_helper.c| 4 > > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c| 2 -- > > drivers/gpu/drm/tegra/dc.c | 1 - > > drivers/gpu/drm/tidss/tidss_crtc.c | 3 +-- > > drivers/gpu/drm/tidss/tidss_kms.c| 4 > > 8 files changed, 9 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > index 56bd938961ee..f33418d6e1a0 100644 > > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > > @@ -492,10 +492,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc) > > crtc->state = NULL; > > > > state = kzalloc(sizeof(*state), GFP_KERNEL); > > - if (state) { > > - crtc->state = >base; > > - crtc->state->crtc = crtc; > > - } > > + if (state) > > + __drm_atomic_helper_crtc_reset(crtc, >base); > > } > > > > static struct drm_crtc_state * > > @@ -616,7 +614,6 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms, > > return err; > > > > drm_crtc_helper_add(crtc, _crtc_helper_funcs); > > - drm_crtc_vblank_reset(crtc); > > > > crtc->port = kcrtc->master->of_output_port; > > > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > > b/drivers/gpu/drm/arm/malidp_drv.c > > index c2507b7d8512..02904392e370 100644 > > --- a/drivers/gpu/drm/arm/malidp_drv.c > > +++ b/drivers/gpu/drm/arm/malidp_drv.c > > @@ -870,7 +870,6 @@ static int malidp_bind(struct device *dev) > > drm->irq_enabled = true; > > > > ret = drm_vblank_init(drm, drm->mode_config.num_crtc); > > - drm_crtc_vblank_reset(>crtc); > > It was a couple of years ago but I remember Alexandru-Cosmin tracking an > issue around > this. The reason for reseting the vblank
Re: [PATCH] drm/atomic-helper: reset vblank on crtc reset
Hi Daniel, On Wed, May 27, 2020 at 11:53:32AM +0200, Daniel Vetter wrote: > Only when vblanks are supported ofc. > > Some drivers do this already, but most unfortunately missed it. This > opens up bugs after driver load, before the crtc is enabled for the > first time. syzbot spotted this when loading vkms as a secondary > output. Given how many drivers are buggy it's best to solve this once > and for all in shared helper code. > > Aside from moving the few existing calls to drm_crtc_vblank_reset into > helpers (i915 doesn't use helpers, so keeps its own) I think the > regression risk is minimal: atomic helpers already rely on drivers > calling drm_crtc_vblank_on/off correctly in their hooks when they > support vblanks. And driver that's failing to handle vblanks after > this is missing those calls already, and vblanks could only work by > accident when enabling a CRTC for the first time right after boot. > > Big thanks to Tetsuo for helping track down what's going wrong here. > > There's only a few drivers which already had the necessary call and > needed some updating: > - komeda, atmel and tidss also needed to be changed to call > __drm_atomic_helper_crtc_reset() intead of open coding it > - tegra and msm even had it in the same place already, just code > motion, and malidp already uses __drm_atomic_helper_crtc_reset(). > > Only call left is in i915, which doesn't use drm_mode_config_reset, > but has its own fastboot infrastructure. So that's the only case where > we actually want this in the driver still. > > I've also reviewed all other drivers which set up vblank support with > drm_vblank_init. After the previous patch fixing mxsfb all atomic > drivers do call drm_crtc_vblank_on/off as they should, the remaining > drivers are either legacy kms or legacy dri1 drivers, so not affected > by this change to atomic helpers. > > v2: Use the drm_dev_has_vblank() helper. > > Link: > https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb > Reported-by: Tetsuo Handa > Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com > Cc: Tetsuo Handa > Cc: "James (Qian) Wang" > Cc: Liviu Dudau > Cc: Mihail Atanassov > Cc: Brian Starkey > Cc: Sam Ravnborg > Cc: Boris Brezillon > Cc: Nicolas Ferre > Cc: Alexandre Belloni > Cc: Ludovic Desroches > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > Cc: Thierry Reding > Cc: Jonathan Hunter > Cc: Jyri Sarha > Cc: Tomi Valkeinen > Cc: Rob Clark > Cc: Sean Paul > Cc: Brian Masney > Cc: Emil Velikov > Cc: zhengbin > Cc: Thomas Gleixner > Cc: linux-te...@vger.kernel.org > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 7 ++- > drivers/gpu/drm/arm/malidp_drv.c | 1 - > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 7 ++- > drivers/gpu/drm/drm_atomic_state_helper.c| 4 > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c| 2 -- > drivers/gpu/drm/tegra/dc.c | 1 - > drivers/gpu/drm/tidss/tidss_crtc.c | 3 +-- > drivers/gpu/drm/tidss/tidss_kms.c| 4 > 8 files changed, 9 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > index 56bd938961ee..f33418d6e1a0 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c > @@ -492,10 +492,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc) > crtc->state = NULL; > > state = kzalloc(sizeof(*state), GFP_KERNEL); > - if (state) { > - crtc->state = >base; > - crtc->state->crtc = crtc; > - } > + if (state) > + __drm_atomic_helper_crtc_reset(crtc, >base); > } > > static struct drm_crtc_state * > @@ -616,7 +614,6 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms, > return err; > > drm_crtc_helper_add(crtc, _crtc_helper_funcs); > - drm_crtc_vblank_reset(crtc); > > crtc->port = kcrtc->master->of_output_port; > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > b/drivers/gpu/drm/arm/malidp_drv.c > index c2507b7d8512..02904392e370 100644 > --- a/drivers/gpu/drm/arm/malidp_drv.c > +++ b/drivers/gpu/drm/arm/malidp_drv.c > @@ -870,7 +870,6 @@ static int malidp_bind(struct device *dev) > drm->irq_enabled = true; > > ret = drm_vblank_init(drm, drm->mode_config.num_crtc); > - drm_crtc_vblank_reset(>crtc); It was a couple of years ago but I remember Alexandru-Cosmin tracking an issue around this. The reason for reseting the vblank so early after calling drm_vblank_init was that you could have userspace calling drm_wait_vblank() before crtc activation and in our case it will trigger some warnings in the driver (details are fuzzy right now). The relevant commit was cabce6343fd ("drm: mali-dp: Call
Re: [PATCH] drm/atomic-helper: reset vblank on crtc reset
On Wed, May 27, 2020 at 11:53:32AM +0200, Daniel Vetter wrote: > Only when vblanks are supported ofc. > > Some drivers do this already, but most unfortunately missed it. This > opens up bugs after driver load, before the crtc is enabled for the > first time. syzbot spotted this when loading vkms as a secondary > output. Given how many drivers are buggy it's best to solve this once > and for all in shared helper code. > > Aside from moving the few existing calls to drm_crtc_vblank_reset into > helpers (i915 doesn't use helpers, so keeps its own) I think the > regression risk is minimal: atomic helpers already rely on drivers > calling drm_crtc_vblank_on/off correctly in their hooks when they > support vblanks. And driver that's failing to handle vblanks after > this is missing those calls already, and vblanks could only work by > accident when enabling a CRTC for the first time right after boot. > > Big thanks to Tetsuo for helping track down what's going wrong here. > > There's only a few drivers which already had the necessary call and > needed some updating: > - komeda, atmel and tidss also needed to be changed to call > __drm_atomic_helper_crtc_reset() intead of open coding it > - tegra and msm even had it in the same place already, just code > motion, and malidp already uses __drm_atomic_helper_crtc_reset(). > > Only call left is in i915, which doesn't use drm_mode_config_reset, > but has its own fastboot infrastructure. So that's the only case where > we actually want this in the driver still. > > I've also reviewed all other drivers which set up vblank support with > drm_vblank_init. After the previous patch fixing mxsfb all atomic > drivers do call drm_crtc_vblank_on/off as they should, the remaining > drivers are either legacy kms or legacy dri1 drivers, so not affected > by this change to atomic helpers. > > v2: Use the drm_dev_has_vblank() helper. > > Link: > https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb > Reported-by: Tetsuo Handa > Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com > Cc: Tetsuo Handa > Cc: "James (Qian) Wang" > Cc: Liviu Dudau > Cc: Mihail Atanassov > Cc: Brian Starkey > Cc: Sam Ravnborg > Cc: Boris Brezillon > Cc: Nicolas Ferre > Cc: Alexandre Belloni > Cc: Ludovic Desroches > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: David Airlie > Cc: Daniel Vetter > Cc: Thierry Reding > Cc: Jonathan Hunter > Cc: Jyri Sarha > Cc: Tomi Valkeinen > Cc: Rob Clark > Cc: Sean Paul > Cc: Brian Masney > Cc: Emil Velikov > Cc: zhengbin > Cc: Thomas Gleixner > Cc: linux-te...@vger.kernel.org > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 7 ++- > drivers/gpu/drm/arm/malidp_drv.c | 1 - > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 7 ++- > drivers/gpu/drm/drm_atomic_state_helper.c| 4 > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c| 2 -- > drivers/gpu/drm/tegra/dc.c | 1 - > drivers/gpu/drm/tidss/tidss_crtc.c | 3 +-- > drivers/gpu/drm/tidss/tidss_kms.c| 4 > 8 files changed, 9 insertions(+), 20 deletions(-) Looks good, and nice cleanup: Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/atomic-helper: reset vblank on crtc reset
Only when vblanks are supported ofc. Some drivers do this already, but most unfortunately missed it. This opens up bugs after driver load, before the crtc is enabled for the first time. syzbot spotted this when loading vkms as a secondary output. Given how many drivers are buggy it's best to solve this once and for all in shared helper code. Aside from moving the few existing calls to drm_crtc_vblank_reset into helpers (i915 doesn't use helpers, so keeps its own) I think the regression risk is minimal: atomic helpers already rely on drivers calling drm_crtc_vblank_on/off correctly in their hooks when they support vblanks. And driver that's failing to handle vblanks after this is missing those calls already, and vblanks could only work by accident when enabling a CRTC for the first time right after boot. Big thanks to Tetsuo for helping track down what's going wrong here. There's only a few drivers which already had the necessary call and needed some updating: - komeda, atmel and tidss also needed to be changed to call __drm_atomic_helper_crtc_reset() intead of open coding it - tegra and msm even had it in the same place already, just code motion, and malidp already uses __drm_atomic_helper_crtc_reset(). Only call left is in i915, which doesn't use drm_mode_config_reset, but has its own fastboot infrastructure. So that's the only case where we actually want this in the driver still. I've also reviewed all other drivers which set up vblank support with drm_vblank_init. After the previous patch fixing mxsfb all atomic drivers do call drm_crtc_vblank_on/off as they should, the remaining drivers are either legacy kms or legacy dri1 drivers, so not affected by this change to atomic helpers. v2: Use the drm_dev_has_vblank() helper. Link: https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb Reported-by: Tetsuo Handa Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com Cc: Tetsuo Handa Cc: "James (Qian) Wang" Cc: Liviu Dudau Cc: Mihail Atanassov Cc: Brian Starkey Cc: Sam Ravnborg Cc: Boris Brezillon Cc: Nicolas Ferre Cc: Alexandre Belloni Cc: Ludovic Desroches Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Cc: Thierry Reding Cc: Jonathan Hunter Cc: Jyri Sarha Cc: Tomi Valkeinen Cc: Rob Clark Cc: Sean Paul Cc: Brian Masney Cc: Emil Velikov Cc: zhengbin Cc: Thomas Gleixner Cc: linux-te...@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 7 ++- drivers/gpu/drm/arm/malidp_drv.c | 1 - drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 7 ++- drivers/gpu/drm/drm_atomic_state_helper.c| 4 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c| 2 -- drivers/gpu/drm/tegra/dc.c | 1 - drivers/gpu/drm/tidss/tidss_crtc.c | 3 +-- drivers/gpu/drm/tidss/tidss_kms.c| 4 8 files changed, 9 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c index 56bd938961ee..f33418d6e1a0 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c @@ -492,10 +492,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc) crtc->state = NULL; state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) { - crtc->state = >base; - crtc->state->crtc = crtc; - } + if (state) + __drm_atomic_helper_crtc_reset(crtc, >base); } static struct drm_crtc_state * @@ -616,7 +614,6 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms, return err; drm_crtc_helper_add(crtc, _crtc_helper_funcs); - drm_crtc_vblank_reset(crtc); crtc->port = kcrtc->master->of_output_port; diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index c2507b7d8512..02904392e370 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -870,7 +870,6 @@ static int malidp_bind(struct device *dev) drm->irq_enabled = true; ret = drm_vblank_init(drm, drm->mode_config.num_crtc); - drm_crtc_vblank_reset(>crtc); if (ret < 0) { DRM_ERROR("failed to initialise vblank\n"); goto vblank_fail; diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 10985134ce0b..ce246b96330b 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -411,10 +411,8 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc) } state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) { - crtc->state = >base; - crtc->state->crtc = crtc; - } + if (state) +
Re: [PATCH v6 8/9] spi: stm32: Add 'SPI_SIMPLEX_RX', 'SPI_3WIRE_RX' support for stm32f4
On Wed, May 27, 2020 at 03:27:32PM +0800, dillon.min...@gmail.com wrote: > From: dillon min > > in l3gd20 driver startup, there is a setup failed error return from > stm32 spi driver Please do not submit new versions of already applied patches, please submit incremental updates to the existing code. Modifying existing commits creates problems for other users building on top of those commits so it's best practice to only change pubished git commits if absolutely essential. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/mxsfb: Call drm_crtc_vblank_on/off
mxsfb has vblank support, is atomic, but doesn't call drm_crtc_vblank_on/off as it should. Not good. With my next patch to add the drm_crtc_vblank_reset to helpers this means not even the very first crtc enabling will vblanks work anymore, since they'll just stay off forever. Since mxsfb doesn't have any vblank waits of its own in the enable/disable flow, nor an enable/disable_vblank callback we can do the on/off as the first respectively last operation, and it should all work. Signed-off-by: Daniel Vetter Cc: Marek Vasut Cc: Stefan Agner Cc: Shawn Guo Cc: Sascha Hauer Cc: Pengutronix Kernel Team Cc: Fabio Estevam Cc: NXP Linux Team Cc: linux-arm-ker...@lists.infradead.org --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 497cf443a9af..1891cd6deb2f 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -124,6 +124,7 @@ static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe, drm_panel_prepare(mxsfb->panel); mxsfb_crtc_enable(mxsfb); drm_panel_enable(mxsfb->panel); + drm_crtc_vblank_on(>crtc); } static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe) @@ -133,6 +134,7 @@ static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe) struct drm_crtc *crtc = >crtc; struct drm_pending_vblank_event *event; + drm_crtc_vblank_off(>crtc); drm_panel_disable(mxsfb->panel); mxsfb_crtc_disable(mxsfb); drm_panel_unprepare(mxsfb->panel); -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/atomic-helper: reset vblank on crtc reset
Only when vblanks are supported ofc. Some drivers do this already, but most unfortunately missed it. This opens up bugs after driver load, before the crtc is enabled for the first time. syzbot spotted this when loading vkms as a secondary output. Given how many drivers are buggy it's best to solve this once and for all in shared helper code. Aside from moving the few existing calls to drm_crtc_vblank_reset into helpers (i915 doesn't use helpers, so keeps its own) I think the regression risk is minimal: atomic helpers already rely on drivers calling drm_crtc_vblank_on/off correctly in their hooks when they support vblanks. And driver that's failing to handle vblanks after this is missing those calls already, and vblanks could only work by accident when enabling a CRTC for the first time right after boot. Big thanks to Tetsuo for helping track down what's going wrong here. There's only a few drivers which already had the necessary call and needed some updating: - komeda, atmel and tidss also needed to be changed to call __drm_atomic_helper_crtc_reset() intead of open coding it - tegra and msm even had it in the same place already, just code motion, and malidp already uses __drm_atomic_helper_crtc_reset(). Only call left is in i915, which doesn't use drm_mode_config_reset, but has its own fastboot infrastructure. So that's the only case where we actually want this in the driver still. I've also reviewed all other drivers which set up vblank support with drm_vblank_init. After the previous patch fixing mxsfb all atomic drivers do call drm_crtc_vblank_on/off as they should, the remaining drivers are either legacy kms or legacy dri1 drivers, so not affected by this change to atomic helpers. Link: https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb Reported-by: Tetsuo Handa Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com Cc: Tetsuo Handa Cc: "James (Qian) Wang" Cc: Liviu Dudau Cc: Mihail Atanassov Cc: Brian Starkey Cc: Sam Ravnborg Cc: Boris Brezillon Cc: Nicolas Ferre Cc: Alexandre Belloni Cc: Ludovic Desroches Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Cc: Thierry Reding Cc: Jonathan Hunter Cc: Jyri Sarha Cc: Tomi Valkeinen Cc: Rob Clark Cc: Sean Paul Cc: Brian Masney Cc: Emil Velikov Cc: zhengbin Cc: Thomas Gleixner Cc: linux-te...@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 7 ++- drivers/gpu/drm/arm/malidp_drv.c | 1 - drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 7 ++- drivers/gpu/drm/drm_atomic_state_helper.c| 4 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c| 2 -- drivers/gpu/drm/tegra/dc.c | 1 - drivers/gpu/drm/tidss/tidss_crtc.c | 3 +-- drivers/gpu/drm/tidss/tidss_kms.c| 4 8 files changed, 9 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c index 56bd938961ee..f33418d6e1a0 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c @@ -492,10 +492,8 @@ static void komeda_crtc_reset(struct drm_crtc *crtc) crtc->state = NULL; state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) { - crtc->state = >base; - crtc->state->crtc = crtc; - } + if (state) + __drm_atomic_helper_crtc_reset(crtc, >base); } static struct drm_crtc_state * @@ -616,7 +614,6 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms, return err; drm_crtc_helper_add(crtc, _crtc_helper_funcs); - drm_crtc_vblank_reset(crtc); crtc->port = kcrtc->master->of_output_port; diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index c2507b7d8512..02904392e370 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -870,7 +870,6 @@ static int malidp_bind(struct device *dev) drm->irq_enabled = true; ret = drm_vblank_init(drm, drm->mode_config.num_crtc); - drm_crtc_vblank_reset(>crtc); if (ret < 0) { DRM_ERROR("failed to initialise vblank\n"); goto vblank_fail; diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 10985134ce0b..ce246b96330b 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -411,10 +411,8 @@ static void atmel_hlcdc_crtc_reset(struct drm_crtc *crtc) } state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) { - crtc->state = >base; - crtc->state->crtc = crtc; - } + if (state) + __drm_atomic_helper_crtc_reset(crtc, >base); } static
Re: [PATCH v2 00/91] drm/vc4: Support BCM2711 Display Pipelin
On Wed, May 27, 2020 at 5:13 PM Maxime Ripard wrote: > I'm about to send a v3 today or tomorrow, I can Cc you (and Jian-Hong) if you > want. That would be great, although given the potentially inconsistent results we've been seeing so far it would be great if you could additionally push a git branch somewhere. That way we can have higher confidence that we are applying exactly the same patches to the same base etc. Thanks Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/fourcc: document modifier uniqueness requirements
On Wed, May 27, 2020 at 08:52:00AM +, Simon Ser wrote: > There have suggestions to bake pitch alignment, address alignement, > contiguous memory or other placement (hidden VRAM, GTT/BAR, etc) > constraints into modifiers. Last time this was brought up it seemed > like the consensus was to not allow this. Document this in drm_fourcc.h. > > There are several reasons for this. > > - Encoding all of these constraints in the modifiers would explode the > search space pretty quickly (we only have 64 bits to work with). > - Modifiers need to be unambiguous: a buffer can only have a single > modifier. > - Modifier users aren't expected to parse modifiers. > > Signed-off-by: Simon Ser > Cc: Daniel Vetter > Cc: Daniel Stone > Cc: Bas Nieuwenhuizen > Cc: Dave Airlie > Cc: Marek Olšák > --- > include/uapi/drm/drm_fourcc.h | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > index 490143500a50..97eb0f1cf9f8 100644 > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -58,6 +58,17 @@ extern "C" { > * may preserve meaning - such as number of planes - from the fourcc code, > * whereas others may not. > * > + * Modifiers must uniquely encode buffer layout. In other words, a buffer > must > + * match only a single modifier. A modifier must not be a subset of layouts > of > + * another modifier. For instance, it's incorrect to encode pitch alignment > in > + * a modifier: a buffer may match a 64-pixel aligned modifier and a 32-pixel > + * aligned modifier. That said, modifiers can have implicit minimal > + * requirements. I think we should also add the aliasing when the fourcc codes are involved, with afbc as example. Maybe: For modifiers where the combination of fourcc code and modifier can alias, a canonical pair needs to be defined and used by all drivers. An example is afbc, where both argb and abgr have the exact same compressed layout. With something like that added: Reviewed-by: Daniel Vetter > + * > + * Users see modifiers as opaque tokens they can check for equality and > + * intersect. Users musn't need to know to reason about the modifier value > + * (i.e. users are not expected to extract information out of the modifier). > + * > * Vendors should document their modifier usage in as much detail as > * possible, to ensure maximum compatibility across devices, drivers and > * applications. > -- > 2.26.2 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Convert get_user_pages() --> pin_user_pages()
On Wed, May 27, 2020 at 10:48:52AM +0200, Daniel Vetter wrote: > On Tue, May 26, 2020 at 03:57:45PM -0700, John Hubbard wrote: > > On 2020-05-26 14:00, Souptick Joarder wrote: > > > This code was using get_user_pages(), in a "Case 2" scenario > > > (DMA/RDMA), using the categorization from [1]. That means that it's > > > time to convert the get_user_pages() + release_pages() calls to > > > pin_user_pages() + unpin_user_pages() calls. > > > > > > There is some helpful background in [2]: basically, this is a small > > > part of fixing a long-standing disconnect between pinning pages, and > > > file systems' use of those pages. > > > > > > [1] Documentation/core-api/pin_user_pages.rst > > > > > > [2] "Explicit pinning of user-space pages": > > > https://lwn.net/Articles/807108/ > > I don't think this is a case 2 here, nor is it any of the others. Feels > like not covered at all by the doc. > > radeon has a mmu notifier (might be a bit broken, but hey whatever there's > other drivers which have the same concept, but less broken). So when you > do an munmap, radeon will release the page refcount. I forgot to add: It's also not case 3, since there's no hw page fault support. It's all faked in software, and explicitly synchronizes against pending io (or preempts it, that depends a bit upon the jobs running). > Which case it that? > > Note that currently only amdgpu doesn't work like that for gpu dma > directly to userspace ranges, it uses hmm and afaiui doens't hold a full > page pin refcount. > > Cheers, Daniel > > > > > > > > Signed-off-by: Souptick Joarder > > > Cc: John Hubbard > > > > > > Hi, > > > > > > I'm compile tested this, but unable to run-time test, so any testing > > > help is much appriciated. > > > --- > > > drivers/gpu/drm/radeon/radeon_ttm.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c > > > b/drivers/gpu/drm/radeon/radeon_ttm.c > > > index 5d50c9e..e927de2 100644 > > > --- a/drivers/gpu/drm/radeon/radeon_ttm.c > > > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > > > @@ -506,7 +506,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt > > > *ttm) > > > uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE; > > > struct page **pages = ttm->pages + pinned; > > > - r = get_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0, > > > + r = pin_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0, > > > pages, NULL); > > > if (r < 0) > > > goto release_pages; > > > @@ -535,7 +535,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt > > > *ttm) > > > kfree(ttm->sg); > > > release_pages: > > > - release_pages(ttm->pages, pinned); > > > + unpin_user_pages(ttm->pages, pinned); > > > return r; > > > } > > > @@ -562,7 +562,7 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt > > > *ttm) > > > set_page_dirty(page); > > > > > > Maybe we also need a preceding patch, to fix the above? It should be > > set_page_dirty_lock(), rather than set_page_dirty(), unless I'm overlooking > > something (which is very possible!). > > > > Either way, from a tunnel vision perspective of changing gup to pup, this > > looks good to me, so > > > > Acked-by: John Hubbard > > > > > > thanks, > > -- > > John Hubbard > > NVIDIA > > > > > mark_page_accessed(page); > > > - put_page(page); > > > + unpin_user_page(page); > > > } > > > sg_free_table(ttm->sg); > > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/fourcc: document modifier uniqueness requirements
There have suggestions to bake pitch alignment, address alignement, contiguous memory or other placement (hidden VRAM, GTT/BAR, etc) constraints into modifiers. Last time this was brought up it seemed like the consensus was to not allow this. Document this in drm_fourcc.h. There are several reasons for this. - Encoding all of these constraints in the modifiers would explode the search space pretty quickly (we only have 64 bits to work with). - Modifiers need to be unambiguous: a buffer can only have a single modifier. - Modifier users aren't expected to parse modifiers. Signed-off-by: Simon Ser Cc: Daniel Vetter Cc: Daniel Stone Cc: Bas Nieuwenhuizen Cc: Dave Airlie Cc: Marek Olšák --- include/uapi/drm/drm_fourcc.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 490143500a50..97eb0f1cf9f8 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -58,6 +58,17 @@ extern "C" { * may preserve meaning - such as number of planes - from the fourcc code, * whereas others may not. * + * Modifiers must uniquely encode buffer layout. In other words, a buffer must + * match only a single modifier. A modifier must not be a subset of layouts of + * another modifier. For instance, it's incorrect to encode pitch alignment in + * a modifier: a buffer may match a 64-pixel aligned modifier and a 32-pixel + * aligned modifier. That said, modifiers can have implicit minimal + * requirements. + * + * Users see modifiers as opaque tokens they can check for equality and + * intersect. Users musn't need to know to reason about the modifier value + * (i.e. users are not expected to extract information out of the modifier). + * * Vendors should document their modifier usage in as much detail as * possible, to ensure maximum compatibility across devices, drivers and * applications. -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Convert get_user_pages() --> pin_user_pages()
On Tue, May 26, 2020 at 03:57:45PM -0700, John Hubbard wrote: > On 2020-05-26 14:00, Souptick Joarder wrote: > > This code was using get_user_pages(), in a "Case 2" scenario > > (DMA/RDMA), using the categorization from [1]. That means that it's > > time to convert the get_user_pages() + release_pages() calls to > > pin_user_pages() + unpin_user_pages() calls. > > > > There is some helpful background in [2]: basically, this is a small > > part of fixing a long-standing disconnect between pinning pages, and > > file systems' use of those pages. > > > > [1] Documentation/core-api/pin_user_pages.rst > > > > [2] "Explicit pinning of user-space pages": > > https://lwn.net/Articles/807108/ I don't think this is a case 2 here, nor is it any of the others. Feels like not covered at all by the doc. radeon has a mmu notifier (might be a bit broken, but hey whatever there's other drivers which have the same concept, but less broken). So when you do an munmap, radeon will release the page refcount. Which case it that? Note that currently only amdgpu doesn't work like that for gpu dma directly to userspace ranges, it uses hmm and afaiui doens't hold a full page pin refcount. Cheers, Daniel > > > > Signed-off-by: Souptick Joarder > > Cc: John Hubbard > > > > Hi, > > > > I'm compile tested this, but unable to run-time test, so any testing > > help is much appriciated. > > --- > > drivers/gpu/drm/radeon/radeon_ttm.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c > > b/drivers/gpu/drm/radeon/radeon_ttm.c > > index 5d50c9e..e927de2 100644 > > --- a/drivers/gpu/drm/radeon/radeon_ttm.c > > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c > > @@ -506,7 +506,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm) > > uint64_t userptr = gtt->userptr + pinned * PAGE_SIZE; > > struct page **pages = ttm->pages + pinned; > > - r = get_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0, > > + r = pin_user_pages(userptr, num_pages, write ? FOLL_WRITE : 0, > >pages, NULL); > > if (r < 0) > > goto release_pages; > > @@ -535,7 +535,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_tt *ttm) > > kfree(ttm->sg); > > release_pages: > > - release_pages(ttm->pages, pinned); > > + unpin_user_pages(ttm->pages, pinned); > > return r; > > } > > @@ -562,7 +562,7 @@ static void radeon_ttm_tt_unpin_userptr(struct ttm_tt > > *ttm) > > set_page_dirty(page); > > > Maybe we also need a preceding patch, to fix the above? It should be > set_page_dirty_lock(), rather than set_page_dirty(), unless I'm overlooking > something (which is very possible!). > > Either way, from a tunnel vision perspective of changing gup to pup, this > looks good to me, so > > Acked-by: John Hubbard > > > thanks, > -- > John Hubbard > NVIDIA > > > mark_page_accessed(page); > > - put_page(page); > > + unpin_user_page(page); > > } > > sg_free_table(ttm->sg); > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
+ more folks On 5/18/2020 9:55 PM, Rob Clark wrote: On Mon, May 18, 2020 at 7:23 AM Jordan Crouse wrote: On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote: This patches replaces the previously used static DDR vote and uses dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling GPU frequency. Signed-off-by: Sharat Masetty --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 2d8124b..79433d3 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) gmu->freq = gmu->gpu_freqs[perf_index]; - /* - * Eventually we will want to scale the path vote with the frequency but - * for now leave it at max so that the performance is nominal. - */ - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); + dev_pm_opp_set_bw(>pdev->dev, opp); } This adds an implicit requirement that all targets need bandwidth settings defined in the OPP or they won't get a bus vote at all. I would prefer that there be an default escape valve but if not you'll need to add bandwidth values for the sdm845 OPP that target doesn't regress. it looks like we could maybe do something like: ret = dev_pm_opp_set_bw(...); if (ret) { dev_warn_once(dev, "no bandwidth settings"); icc_set_bw(...); } ? BR, -R There is a bit of an issue here - Looks like its not possible to two icc handles to the same path. Its causing double enumeration of the paths in the icc core and messing up path votes. With [1] Since opp/core already gets a handle to the icc path as part of table add, drm/msm could do either a) Conditionally enumerate gpu->icc_path handle only when pm/opp core has not got the icc path handle. I could use something like [2] to determine if should initialize gpu->icc_path* b) Add peak-opp-configs in 845 dt and mandate all future versions to use this bindings. With this, I can remove gpu->icc_path from msm/drm completely and only rely on opp/core for bw voting. [1] - https://lore.kernel.org/patchwork/cover/1240687/ [2] - https://patchwork.kernel.org/patch/11527573/ Let me know your thoughts Sharat Jordan unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) -- 2.7.4 -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ Freedreno mailing list freedr...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 5/9] clk: stm32: Fix stm32f429's ltdc driver hang in set clock rate
Quoting dillon.min...@gmail.com (2020-05-27 00:27:29) > From: dillon min > > This is due to misuse \u2018PLL_VCO_SAI' and'PLL_SAI' in clk-stm32f4.c > 'PLL_SAI' is 2, 'PLL_VCO_SAI' is 7(defined in > include/dt-bindings/clock/stm32fx-clock.h). > > 'post_div' point to 'post_div_data[]', 'post_div->pll_num' > is PLL_I2S or PLL_SAI. > > 'clks[PLL_VCO_SAI]' has valid 'struct clk_hw* ' return > from stm32f4_rcc_register_pll() but, at line 1777 of > driver/clk/clk-stm32f4.c, use the 'clks[post_div->pll_num]', > equal to 'clks[PLL_SAI]', this is invalid array member at that time. > > Fixes: 517633ef630e ("clk: stm32f4: Add post divisor for I2S & SAI PLLs") > Signed-off-by: dillon min > --- Acked-by: Stephen Boyd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 6/9] clk: stm32: Fix ltdc's clock turn off by clk_disable_unused() after kernel startup
Quoting dillon.min...@gmail.com (2020-05-27 00:27:30) > From: dillon min > > stm32's clk driver register two ltdc gate clk to clk core by > clk_hw_register_gate() and clk_hw_register_composite() > > first: 'stm32f429_gates[]', clk name is 'ltdc', which no user to use. > second: 'stm32f429_aux_clk[]', clk name is 'lcd-tft', used by ltdc driver > > both of them point to the same offset of stm32's RCC register. after > kernel enter console, clk core turn off ltdc's clk as 'stm32f429_gates[]' > is no one to use. but, actually 'stm32f429_aux_clk[]' is in use. > > Fixes: daf2d117cbca ("clk: stm32f4: Add lcd-tft clock") > Signed-off-by: dillon min > --- Acked-by: Stephen Boyd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vkms: Don't warn hrtimer_forward_now failure.
On Tue, May 26, 2020 at 01:11:28PM +0200, Daniel Vetter wrote: > On Tue, May 26, 2020 at 6:39 AM Tetsuo Handa > wrote: > > > > On 2020/05/26 13:18, Tetsuo Handa wrote: > > > due to mode->crtc_clock <= 0. Thus, somehow initializing mode->crtc_clock > > > > 0 might be able > > > to solve this problem. > > > > Well, I came to think that vkms_enable_vblank() should return an error to > > the caller > > when drm_calc_timestamping_constants() failed... > > If my memory is right we shouldn't even get there. crtc->mode being > all zeros sounds like the simulated output isn't on (no surprise, > syzbot doesn't enable it and fbcon is probably on card0), so something > higher up should reject this. I'll see whether I can figure out what > vkms isn't doing right (or whether there's a higher level bug in > drm_vblank.c code), yesterday my machine died and already evening > anyway. > > Thanks for digging into this a bit more meanwhile. Ok good news, I think I have a fix. Bad news it's code used by like 50 drivers, so this will take some time to get reviewed and merged (and I think almost all these other drivers are buggy too, which is why I decided to fix the shared code). Patch below, can you pls confirm this fixes it? Thanks, Daniel commit b882894ac0fce412b67db79a5c92f2b599ec5069 Author: Daniel Vetter Date: Tue May 26 14:29:00 2020 +0200 drm/atomic-helper: reset vblank on crtc reset Only when vblanks are supported ofc. Some drivers do this already, but most unfortunately missed it. This opens up bugs after driver load, before the crtc is enabled for the first time. syzbot spotted this when loading vkms as a secondary output. Given how many drivers are buggy it's best to solve this once and for all in shared helper code. Aside from moving the few existing calls to drm_crtc_vblank_reset into helpers (i915 doesn't use helpers, so keeps its own) I think the regression risk is minimal: atomic helpers already rely on drivers calling drm_crtc_vblank_on/off correctly in their hooks when they support vblanks. And driver that's failing to handle vblanks after this is missing those calls already, and vblanks could only work by accident when enabling a CRTC for the first time right after boot. FIXME: Fix up drivers and audit them all. Big thanks to Tetsuo for helping track down what's going wrong here. Link: https://syzkaller.appspot.com/bug?id=0ba17d70d062b2595e1f061231474800f076c7cb Reported-by: Tetsuo Handa Reported-by: syzbot+0871b14ca2e2fb64f...@syzkaller.appspotmail.com Cc: sta...@vger.kernel.org Cc: Tetsuo Handa Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index 8fce6a115dfe..843964252239 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -93,6 +94,9 @@ __drm_atomic_helper_crtc_reset(struct drm_crtc *crtc, if (crtc_state) __drm_atomic_helper_crtc_state_reset(crtc_state, crtc); + if (crtc->dev->num_crtcs) + drm_crtc_vblank_reset(crtc); + crtc->state = crtc_state; } EXPORT_SYMBOL(__drm_atomic_helper_crtc_reset); -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next-fixes
Hi Dave and Daniel, here's the pull request for the current drm-misc-next-fixes. Best regards Thomas drm-misc-next-fixes-2020-05-27: Short summary of fixes pull (less than what git shortlog provides): There's a fix for panel brighness on Lenovo X13 Yoga devices and a fix for -Wformat warnings on architectures where atomic-64 counters are not of type unsigned long long. The following changes since commit 152cce0006abf7e17dfb7dc94896b044bda4e588: drm/bridge: analogix_dp: Split bind() into probe() and real bind() (2020-04-09 10:29:35 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2020-05-27 for you to fetch changes up to 6f27e4c287d7bdcad1f24efcaace044617aac2f3: drm/vblank: Fix -Wformat compile warnings on some arches (2020-05-22 14:22:00 -0400) Short summary of fixes pull (less than what git shortlog provides): There's a fix for panel brighness on Lenovo X13 Yoga devices and a fix for -Wformat warnings on architectures where atomic-64 counters are not of type unsigned long long. Lyude Paul (1): drm/vblank: Fix -Wformat compile warnings on some arches Mark Pearson (1): drm/dp: Lenovo X13 Yoga OLED panel brightness fix drivers/gpu/drm/drm_dp_helper.c | 1 + drivers/gpu/drm/drm_vblank.c| 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 0/8] drm: rcar-du: Add Color Management Module (CMM)
Hi Eugeniu, On Wed, May 27, 2020 at 9:16 AM Eugeniu Rosca wrote: > On Tue, Oct 15, 2019 at 12:46:13PM +0200, Jacopo Mondi wrote: > > CMM functionalities are retained between suspend/resume cycles (tested with > > suspend-to-idle) without requiring a re-programming of the LUT tables. > > Hmm. Is this backed up by any statement in the HW User's manual? > This comes in contrast with the original Renesas CMM implementation [**] > which does make use of suspend (where the freeze actually happens). > > Can we infer, based on your statement, that we could also get rid of > the suspend callback in [**]? While the CMM state will be retained across suspend-to-idle, I'm quite sure it will be lost by suspend-to-RAM, at least on the Salvator-X(S), ULCB, and Ebisu development boards, as PSCI will ask the BD9571WMV regulator to power down the R-Car SoC. So IMHO we do need suspend/resume handling. 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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 2/3] virtio-gpu: add VIRTIO_GPU_F_RESOURCE_UUID feature
This feature allows the guest to request a UUID from the host for a particular virtio_gpu resource. The UUID can then be shared with other virtio devices, to allow the other host devices to access the virtio_gpu's corresponding host resource. Signed-off-by: David Stevens --- include/uapi/linux/virtio_gpu.h | 19 +++ 1 file changed, 19 insertions(+) diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h index 0c85914d9369..9721d58b4d58 100644 --- a/include/uapi/linux/virtio_gpu.h +++ b/include/uapi/linux/virtio_gpu.h @@ -50,6 +50,10 @@ * VIRTIO_GPU_CMD_GET_EDID */ #define VIRTIO_GPU_F_EDID1 +/* + * VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID + */ +#define VIRTIO_GPU_F_RESOURCE_UUID 2 enum virtio_gpu_ctrl_type { VIRTIO_GPU_UNDEFINED = 0, @@ -66,6 +70,7 @@ enum virtio_gpu_ctrl_type { VIRTIO_GPU_CMD_GET_CAPSET_INFO, VIRTIO_GPU_CMD_GET_CAPSET, VIRTIO_GPU_CMD_GET_EDID, + VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID, /* 3d commands */ VIRTIO_GPU_CMD_CTX_CREATE = 0x0200, @@ -87,6 +92,7 @@ enum virtio_gpu_ctrl_type { VIRTIO_GPU_RESP_OK_CAPSET_INFO, VIRTIO_GPU_RESP_OK_CAPSET, VIRTIO_GPU_RESP_OK_EDID, + VIRTIO_GPU_RESP_OK_RESOURCE_UUID, /* error responses */ VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, @@ -340,4 +346,17 @@ enum virtio_gpu_formats { VIRTIO_GPU_FORMAT_R8G8B8X8_UNORM = 134, }; +/* VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID */ +struct virtio_gpu_resource_assign_uuid { + struct virtio_gpu_ctrl_hdr hdr; + __le32 resource_id; + __le32 padding; +}; + +/* VIRTIO_GPU_RESP_OK_RESOURCE_UUID */ +struct virtio_gpu_resp_resource_uuid { + struct virtio_gpu_ctrl_hdr hdr; + __u8 uuid[16]; +}; + #endif -- 2.27.0.rc0.183.gde8f92d652-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] tee: convert get_user_pages() --> pin_user_pages()
Hi John, On Tue, May 26, 2020 at 1:32 AM John Hubbard wrote: > > This code was using get_user_pages*(), in a "Case 2" scenario > (DMA/RDMA), using the categorization from [1]. That means that it's > time to convert the get_user_pages*() + put_page() calls to > pin_user_pages*() + unpin_user_pages() calls. > > There is some helpful background in [2]: basically, this is a small > part of fixing a long-standing disconnect between pinning pages, and > file systems' use of those pages. > > [1] Documentation/core-api/pin_user_pages.rst > > [2] "Explicit pinning of user-space pages": > https://lwn.net/Articles/807108/ > > Cc: Jens Wiklander > Cc: Sumit Semwal > Cc: tee-...@lists.linaro.org > Cc: linux-me...@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: linaro-mm-...@lists.linaro.org > Signed-off-by: John Hubbard > --- > > Hi, > > This fixes the typo ("convert convert") in the subject line, but > otherwise no changes. > > thanks, > John Hubbard > NVIDIA > > > drivers/tee/tee_shm.c | 12 +++- > 1 file changed, 3 insertions(+), 9 deletions(-) Looks good. I've tested this on a HiKey 620 board, no regressions. I'm picking up this patch. Thanks, Jens > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > index bd679b72bd05..7dffc42d8d5a 100644 > --- a/drivers/tee/tee_shm.c > +++ b/drivers/tee/tee_shm.c > @@ -31,16 +31,13 @@ static void tee_shm_release(struct tee_shm *shm) > > poolm->ops->free(poolm, shm); > } else if (shm->flags & TEE_SHM_REGISTER) { > - size_t n; > int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm); > > if (rc) > dev_err(teedev->dev.parent, > "unregister shm %p failed: %d", shm, rc); > > - for (n = 0; n < shm->num_pages; n++) > - put_page(shm->pages[n]); > - > + unpin_user_pages(shm->pages, shm->num_pages); > kfree(shm->pages); > } > > @@ -226,7 +223,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, > unsigned long addr, > goto err; > } > > - rc = get_user_pages_fast(start, num_pages, FOLL_WRITE, shm->pages); > + rc = pin_user_pages_fast(start, num_pages, FOLL_WRITE, shm->pages); > if (rc > 0) > shm->num_pages = rc; > if (rc != num_pages) { > @@ -271,16 +268,13 @@ struct tee_shm *tee_shm_register(struct tee_context > *ctx, unsigned long addr, > return shm; > err: > if (shm) { > - size_t n; > - > if (shm->id >= 0) { > mutex_lock(>mutex); > idr_remove(>idr, shm->id); > mutex_unlock(>mutex); > } > if (shm->pages) { > - for (n = 0; n < shm->num_pages; n++) > - put_page(shm->pages[n]); > + unpin_user_pages(shm->pages, shm->num_pages); > kfree(shm->pages); > } > } > -- > 2.26.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/3] virtio: add dma-buf support for exported objects
This change adds a new flavor of dma-bufs that can be used by virtio drivers to share exported objects. A virtio dma-buf can be queried by virtio drivers to obtain the UUID which identifies the underlying exported object. Signed-off-by: David Stevens --- drivers/virtio/Makefile | 2 +- drivers/virtio/virtio.c | 6 +++ drivers/virtio/virtio_dma_buf.c | 89 + include/linux/virtio.h | 1 + include/linux/virtio_dma_buf.h | 58 + 5 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 drivers/virtio/virtio_dma_buf.c create mode 100644 include/linux/virtio_dma_buf.h diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile index 29a1386ecc03..ecdae5b596de 100644 --- a/drivers/virtio/Makefile +++ b/drivers/virtio/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o +obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o virtio_dma_buf.o obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index a977e32a88f2..5d46f0ded92d 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -357,6 +357,12 @@ int register_virtio_device(struct virtio_device *dev) } EXPORT_SYMBOL_GPL(register_virtio_device); +bool is_virtio_device(struct device *dev) +{ + return dev->bus == _bus; +} +EXPORT_SYMBOL_GPL(is_virtio_device); + void unregister_virtio_device(struct virtio_device *dev) { int index = dev->index; /* save for after device release */ diff --git a/drivers/virtio/virtio_dma_buf.c b/drivers/virtio/virtio_dma_buf.c new file mode 100644 index ..23e3399b11ed --- /dev/null +++ b/drivers/virtio/virtio_dma_buf.c @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * dma-bufs for virtio exported objects + * + * Copyright (C) 2020 Google, Inc. + */ + +#include + +/** + * virtio_dma_buf_export - Creates a new dma-buf for a virtio exported object + * + * This wraps dma_buf_export() to allow virtio drivers to create a dma-buf + * for an virtio exported object that can be queried by other virtio drivers + * for the object's UUID. + */ +struct dma_buf *virtio_dma_buf_export( + const struct virtio_dma_buf_export_info *virtio_exp_info) +{ + struct dma_buf_export_info exp_info; + + if (!virtio_exp_info->ops + || virtio_exp_info->ops->ops.attach != _dma_buf_attach + || !virtio_exp_info->ops->get_uuid) { + return ERR_PTR(-EINVAL); + } + + exp_info.exp_name = virtio_exp_info->exp_name; + exp_info.owner = virtio_exp_info->owner; + exp_info.ops = _exp_info->ops->ops; + exp_info.size = virtio_exp_info->size; + exp_info.flags = virtio_exp_info->flags; + exp_info.resv = virtio_exp_info->resv; + exp_info.priv = virtio_exp_info->priv; + BUILD_BUG_ON(sizeof(struct virtio_dma_buf_export_info) +!= sizeof(struct dma_buf_export_info)); + + return dma_buf_export(_info); +} +EXPORT_SYMBOL(virtio_dma_buf_export); + +/** + * virtio_dma_buf_attach - mandatory attach callback for virtio dma-bufs + */ +int virtio_dma_buf_attach(struct dma_buf *dma_buf, + struct dma_buf_attachment *attach) +{ + int ret; + const struct virtio_dma_buf_ops *ops = container_of( + dma_buf->ops, const struct virtio_dma_buf_ops, ops); + + if (ops->device_attach) { + ret = ops->device_attach(dma_buf, attach); + if (ret) + return ret; + } + return 0; +} +EXPORT_SYMBOL(virtio_dma_buf_attach); + +/** + * is_virtio_dma_buf - returns true if the given dma-buf is a virtio dma-buf + * @dma_buf: buffer to query + */ +bool is_virtio_dma_buf(struct dma_buf *dma_buf) +{ + return dma_buf->ops->attach == _dma_buf_attach; +} +EXPORT_SYMBOL(is_virtio_dma_buf); + +/** + * virtio_dma_buf_get_uuid - gets the uuid of the virtio dma-buf's exported object + * @dma_buf: [in] buffer to query + * @uuid: [out] the uuid + * + * Returns: 0 on success, negative on failure. + */ +int virtio_dma_buf_get_uuid(struct dma_buf *dma_buf, + uuid_t *uuid) +{ + const struct virtio_dma_buf_ops *ops = container_of( + dma_buf->ops, const struct virtio_dma_buf_ops, ops); + + if (!is_virtio_dma_buf(dma_buf)) + return -EINVAL; + + return ops->get_uuid(dma_buf, uuid); +} +EXPORT_SYMBOL(virtio_dma_buf_get_uuid); diff --git a/include/linux/virtio.h b/include/linux/virtio.h index 15f906e4a748..9397e25616c4 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -128,6 +128,7 @@ static inline struct virtio_device *dev_to_virtio(struct device *_dev) void virtio_add_status(struct virtio_device *dev, unsigned
Re: [PATCH 09/21] drm/ingenic: Use GEM CMA object functions
Hi Thomas, Le ven. 22 mai 2020 à 15:52, Thomas Zimmermann a écrit : The ingenic driver uses the default implementation for CMA functions. The DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver. All remaining operations are provided by CMA GEM object functions. Signed-off-by: Thomas Zimmermann Tested-by: Paul Cercueil Reviewed-by: Paul Cercueil Cheers, -Paul --- drivers/gpu/drm/ingenic/ingenic-drm.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c index eff57a1f70fb0..1c1cee367b752 100644 --- a/drivers/gpu/drm/ingenic/ingenic-drm.c +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c @@ -519,18 +519,7 @@ static struct drm_driver ingenic_drm_driver_data = { .patchlevel = 0, .fops = _drm_fops, - - .dumb_create= drm_gem_cma_dumb_create, - .gem_free_object_unlocked = drm_gem_cma_free_object, - .gem_vm_ops = _gem_cma_vm_ops, - - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, - .gem_prime_vmap = drm_gem_cma_prime_vmap, - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, - .gem_prime_mmap = drm_gem_cma_prime_mmap, + DRM_GEM_CMA_DRIVER_OPS, .irq_handler= ingenic_drm_irq_handler, }; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 5/8] clk: stm32: Fix stm32f429's ltdc driver hang in set clock rate, fix duplicated ltdc clock register to 'clk_core' case ltdc's clock turn off by clk_disable_unused()
Hi Stephen, Thanks for reviewing. On Wed, May 27, 2020 at 9:44 AM Stephen Boyd wrote: > > Quoting dillon.min...@gmail.com (2020-05-24 20:45:45) > > From: dillon min > > > > ltdc set clock rate crashed > >'post_div_data[]''s pll_num is PLL_I2S, PLL_SAI (number is 1,2). but, > > Please write "post_div_data[]'s" if it is possessive. "But" doesn't > start a sentence. This is one sentence, not two. Ok. > > > as pll_num is offset of 'clks[]' input to clk_register_pll_div(), which > > is FCLK, CLK_LSI, defined in 'include/dt-bindings/clock/stm32fx-clock.h' > > so, this is a null object at the register time. > > then, in ltdc's clock is_enabled(), enable(), will call to_clk_gate(). > > will return a null object, cause kernel crashed. > > need change pll_num to PLL_VCO_I2S, PLL_VCO_SAI for 'post_div_data[]' > > > > duplicated ltdc clock > >'stm32f429_gates[]' has a member 'ltdc' register to 'clk_core', but no > > upper driver use it, ltdc driver use the lcd-tft defined in > >'stm32f429_aux_clk[]'. after system startup, as stm32f429_gates[]'s ltdc > > enable_count is zero, so turn off by clk_disable_unused() > > I sort of follow this. Is this another patch? Seems like two things are > going on here. This patch fix two bugs about stm32's clock. bug1: ltdc driver loading hang in clk_set_rate(), this is due to misuse ‘PLL_VCO_SAI' and 'PLL_SAI'. speak in short, from the below code, ’PLL_SAI' is 2, 'PLL_VCO_SAI' is 7. 'post_div' point to 'post_div_data[]', 'post_div->pll_num' is PLL_I2S, PLL_SAI. 'clks[PLL_VCOM_SAI' has vaild 'struct clk_hw* ' return from stm32f4_rcc_register_pll() but, at line 1776, use the 'clks[post_div->pll_num]', equal to 'clks[PLL_SAI]', this is invaild at that time. include/dt-bindings/clock/stm32fx-clock.h 29 #define PLL_VCO_SAI 7 drivers/clk/clk-stm32f4.c 494 enum { 495 PLL, 496 PLL_I2S, 497 PLL_SAI, 498 }; 558 static const struct stm32f4_pll_post_div_data post_div_data[MAX_POST_DIV] = { 559 { CLK_I2SQ_PDIV, PLL_I2S, "plli2s-q-div", "plli2s-q", 560 CLK_SET_RATE_PARENT, STM32F4_RCC_DCKCFGR, 0, 5, 0, NULL}, 561 562 { CLK_SAIQ_PDIV, PLL_SAI, "pllsai-q-div", "pllsai-q", 563 CLK_SET_RATE_PARENT, STM32F4_RCC_DCKCFGR, 8, 5, 0, NULL }, 564 565 { NO_IDX, PLL_SAI, "pllsai-r-div", "pllsai-r", CLK_SET_RATE_PARENT, 566 STM32F4_RCC_DCKCFGR, 16, 2, 0, post_divr_table }, 567 }; 1759 clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll("vco_in", 1760 >pll_data[2], _clk_lock); 1761 1762 for (n = 0; n < MAX_POST_DIV; n++) { 1763 const struct stm32f4_pll_post_div_data *post_div; 1764 struct clk_hw *hw; 1765 1766 post_div = _div_data[n]; 1767 1768 hw = clk_register_pll_div(post_div->name, 1769 post_div->parent, 1770 post_div->flag, 1771 base + post_div->offset, 1772 post_div->shift, 1773 post_div->width, 1774 post_div->flag_div, 1775 post_div->div_table, 1776 clks[post_div->pll_num], 1777 _clk_lock); 1778 1779 if (post_div->idx != NO_IDX) 1780 clks[post_div->idx] = hw; 1781 } bug2: ltdc's clock turn off by clk_disable_unused() from your comments at '[PATCH v3 4/5] clk: stm32: Fix stm32f429 ltdc driver loading hang in clk set rate. keep ltdc clk running after kernel startup' , i go deep into the code, found stm32's clk driver register two gate clk to clk core by clk_hw_register_gate() and clk_hw_register_composite() first: 'stm32f429_gates[]', clk name is 'ltdc', this is no user used. second: 'stm32f429_aux_clk[]', clk name is 'lcd-tft', this is used by ltdc driver both of them point to the same offset of stm32's RCC register. after kernel enter console, clk core turn off ltdc's clk as 'stm32f429_gates[]' is unused. but, actually 'stm32f429_aux_clk[]' is in use. i can separate this patch to two, each bug a patch if necessary > > > > > Changes since V3: > > 1 drop last wrong changes about 'CLK_IGNORE_UNUSED' patch > > 2 fix PLL_SAI mismatch with PLL_VCO_SAI > > This change log goes under the --- below. ok > > > > > Signed-off-by: dillon min > > Any Fixes tag? ok, will add --fixup in git commit next time, this patch fix two bugs, i should make two commit, each one has a fixes tag, right? first point to '517633e clk: stm32f4: Add post divisor for I2S & SAI PLLs' second point to 'daf2d11 clk: stm32f4: Add lcd-tft clock' ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 3/3] drm/virtio: Support virtgpu exported resources
Add support for UUID-based resource sharing mechanism to virtgpu. This implements the new virtgpu commands and hooks them up to dma-buf's get_uuid callback. Signed-off-by: David Stevens --- drivers/gpu/drm/virtio/virtgpu_drv.c | 3 + drivers/gpu/drm/virtio/virtgpu_drv.h | 20 ++ drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++ drivers/gpu/drm/virtio/virtgpu_prime.c | 96 +- drivers/gpu/drm/virtio/virtgpu_vq.c| 55 +++ 5 files changed, 175 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c index ab4bed78e656..b039f493bda9 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.c +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c @@ -165,6 +165,7 @@ static unsigned int features[] = { VIRTIO_GPU_F_VIRGL, #endif VIRTIO_GPU_F_EDID, + VIRTIO_GPU_F_RESOURCE_UUID, }; static struct virtio_driver virtio_gpu_driver = { .feature_table = features, @@ -202,6 +203,8 @@ static struct drm_driver driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, .gem_prime_mmap = drm_gem_prime_mmap, + .gem_prime_export = virtgpu_gem_prime_export, + .gem_prime_import = virtgpu_gem_prime_import, .gem_prime_import_sg_table = virtgpu_gem_prime_import_sg_table, .gem_create_object = virtio_gpu_create_object, diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 49bebdee6d91..39dc907aa805 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -49,6 +49,10 @@ #define DRIVER_MINOR 1 #define DRIVER_PATCHLEVEL 0 +#define UUID_INITIALIZING 0 +#define UUID_INITIALIZED 1 +#define UUID_INITIALIZATION_FAILED 2 + struct virtio_gpu_object_params { uint32_t format; uint32_t width; @@ -71,6 +75,9 @@ struct virtio_gpu_object { uint32_t hw_res_handle; bool dumb; bool created; + + int uuid_state; + uuid_t uuid; }; #define gem_to_virtio_gpu_obj(gobj) \ container_of((gobj), struct virtio_gpu_object, base.base) @@ -200,6 +207,7 @@ struct virtio_gpu_device { bool has_virgl_3d; bool has_edid; bool has_indirect; + bool has_resource_assign_uuid; struct work_struct config_changed_work; @@ -210,6 +218,8 @@ struct virtio_gpu_device { struct virtio_gpu_drv_capset *capsets; uint32_t num_capsets; struct list_head cap_cache; + + spinlock_t resource_export_lock; }; struct virtio_gpu_fpriv { @@ -335,6 +345,10 @@ void virtio_gpu_dequeue_fence_func(struct work_struct *work); void virtio_gpu_notify(struct virtio_gpu_device *vgdev); +int +virtio_gpu_cmd_resource_assign_uuid(struct virtio_gpu_device *vgdev, + struct virtio_gpu_object_array *objs); + /* virtgpu_display.c */ void virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev); void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev); @@ -366,6 +380,12 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo); /* virtgpu_prime.c */ +struct dma_buf *virtgpu_gem_prime_export(struct drm_gem_object *obj, +int flags); +struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev, + struct dma_buf *buf); +int virtgpu_gem_prime_get_uuid(struct drm_gem_object *obj, + uuid_t *uuid); struct drm_gem_object *virtgpu_gem_prime_import_sg_table( struct drm_device *dev, struct dma_buf_attachment *attach, struct sg_table *sgt); diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 023a030ca7b9..7bcd0c75effa 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -125,6 +125,7 @@ int virtio_gpu_init(struct drm_device *dev) vgdev->dev = dev->dev; spin_lock_init(>display_info_lock); + spin_lock_init(>resource_export_lock); ida_init(>ctx_id_ida); ida_init(>resource_ida); init_waitqueue_head(>resp_wq); @@ -153,6 +154,9 @@ int virtio_gpu_init(struct drm_device *dev) if (virtio_has_feature(vgdev->vdev, VIRTIO_RING_F_INDIRECT_DESC)) { vgdev->has_indirect = true; } + if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_RESOURCE_UUID)) { + vgdev->has_resource_assign_uuid = true; + } DRM_INFO("features: %cvirgl %cedid\n", vgdev->has_virgl_3d ? '+' : '-', diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c index 050d24c39a8f..a753bb70fcf1 100644 --- a/drivers/gpu/drm/virtio/virtgpu_prime.c +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c @@ -23,12 +23,102 @@ */ #include +#include
Re: [PATCH v2 0/2] powerpc: Remove support for ppc405/440 Xilinx platforms
Michal Simek writes: > Hi Michael, > > On 01. 04. 20 13:30, Michal Simek wrote: >> On 01. 04. 20 12:38, Takashi Iwai wrote: >>> On Wed, 01 Apr 2020 12:35:16 +0200, >>> Michael Ellerman wrote: Michal Simek writes: > On 01. 04. 20 4:07, Michael Ellerman wrote: >> Michal Simek writes: >>> Hi, >>> >>> recently we wanted to update xilinx intc driver and we found that >>> function >>> which we wanted to remove is still wired by ancient Xilinx PowerPC >>> platforms. Here is the thread about it. >>> https://lore.kernel.org/linux-next/48d3232d-0f1d-42ea-3109-f44bbabfa...@xilinx.com/ >>> >>> I have been talking about it internally and there is no interest in >>> these >>> platforms and it is also orphan for quite a long time. None is really >>> running/testing these platforms regularly that's why I think it makes >>> sense >>> to remove them also with drivers which are specific to this platform. >>> >>> U-Boot support was removed in 2017 without anybody complain about it >>> https://github.com/Xilinx/u-boot-xlnx/commit/98f705c9cefdfdba62c069821bbba10273a0a8ed >>> >>> Based on current ppc/next. >>> >>> If anyone has any objection about it, please let me know. >> >> Thanks for taking the time to find all this code and remove it. >> >> I'm not going to take this series for v5.7, it was posted too close to >> the merge window, and doing so wouldn't give people much time to object, >> especially given people are distracted at the moment. >> >> I'm happy to take it for v5.8, assuming there's no major objections. > > Sure. Just to let you know Christophe Leroy included this patch in his > series about ppc405 removal. It should be the same. > > If you don't want to take that alsa patch I can send it separately and > this patch can be taken from his series. I don't really mind but please > let me know what way you prefer. It's better to keep it all together, so I'm happy take the alsa patch as well, it's already been acked. > > Can you please take this series? I know that there is v5 from Christophe > which has this 1/2 as 1/13. But I need this alsa patch too and I would > like to close this because it is around for almost 2 months and none > raised a concern about removing just these Xilinx platforms. Sorry I meant to reply to your last mail. I have Christophe's series in my testing branch, planning for it to be in v5.8. Even if the rest of his series doesn't make it for some reason, as you say the Xilinx removal is uncontroversial so I'll keep that in. I forgot about the sound patch, I'll pick that up as well. cheers ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel