[PATCH v3] drm/mediatek: clear pending flag when cmdq packet is done.
In cmdq mode, packet may be flushed before it is executed, so the pending flag should be cleared after cmdq packet is done. Signed-off-by: Yongqiang Niu --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 51 + 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 4c25e33..2a2d43e 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -267,6 +267,40 @@ static void ddp_cmdq_cb(struct mbox_client *cl, void *mssg) { struct mtk_drm_crtc *mtk_crtc = container_of(cl, struct mtk_drm_crtc, cmdq_cl); struct cmdq_cb_data *data = mssg; + struct mtk_crtc_state *state; + unsigned int i; + + state = to_mtk_crtc_state(mtk_crtc->base.state); + + if (state->pending_config) { + state->pending_config = false; + } + + if (mtk_crtc->pending_planes) { + for (i = 0; i < mtk_crtc->layer_nr; i++) { + struct drm_plane *plane = &mtk_crtc->planes[i]; + struct mtk_plane_state *plane_state; + + plane_state = to_mtk_plane_state(plane->state); + + if (plane_state->pending.config) + plane_state->pending.config = false; + } + mtk_crtc->pending_planes = false; + } + + if (mtk_crtc->pending_async_planes) { + for (i = 0; i < mtk_crtc->layer_nr; i++) { + struct drm_plane *plane = &mtk_crtc->planes[i]; + struct mtk_plane_state *plane_state; + + plane_state = to_mtk_plane_state(plane->state); + + if (plane_state->pending.async_config) + plane_state->pending.async_config = false; + } + mtk_crtc->pending_async_planes = false; + } mtk_crtc->cmdq_vblank_cnt = 0; mtk_drm_cmdq_pkt_destroy(mtk_crtc->cmdq_chan, data->pkt); @@ -423,7 +457,8 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, state->pending_vrefresh, 0, cmdq_handle); - state->pending_config = false; + if (!cmdq_handle) + state->pending_config = false; } if (mtk_crtc->pending_planes) { @@ -443,9 +478,12 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, mtk_ddp_comp_layer_config(comp, local_layer, plane_state, cmdq_handle); - plane_state->pending.config = false; + if (!cmdq_handle) + plane_state->pending.config = false; } - mtk_crtc->pending_planes = false; + + if (!cmdq_handle) + mtk_crtc->pending_planes = false; } if (mtk_crtc->pending_async_planes) { @@ -465,9 +503,12 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc, mtk_ddp_comp_layer_config(comp, local_layer, plane_state, cmdq_handle); - plane_state->pending.async_config = false; + if (!cmdq_handle) + plane_state->pending.async_config = false; } - mtk_crtc->pending_async_planes = false; + + if (!cmdq_handle) + mtk_crtc->pending_async_planes = false; } } -- 1.8.1.1.dirty
[PATCH v3] drm/mediatek: clear pending flag when cmdq packet is done
Change since v2: rebase https://patchwork.kernel.org/project/linux-mediatek/cover/20210712235014.42673-1-chunkuang...@kernel.org/ Yongqiang Niu (1): drm/mediatek: clear pending flag when cmdq packet is done. drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 51 + 1 file changed, 46 insertions(+), 5 deletions(-) -- 1.8.1.1.dirty
[PATCH v2] drm: document drm_mode_get_property
It's not obvious what the fields mean and how they should be used. The most important detail is the link to drm_property.flags, which describes how property types work. v2: document enum drm_mode_property_enum, add ref to "Modeset Base Object Abstraction" (Daniel) Signed-off-by: Simon Ser Acked-by: Pekka Paalanen Cc: Daniel Vetter Cc: Leandro Ribeiro --- I couldn't figure out how to linkify that ref, so it's not linkified. Documentation/gpu/drm-kms.rst | 2 ++ include/uapi/drm/drm_mode.h | 60 --- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 0cc21f6aaef5..1ef7951ded5e 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -159,6 +159,8 @@ KMS Core Structures and Functions .. kernel-doc:: drivers/gpu/drm/drm_mode_config.c :export: +.. _kms_base_object_abstraction: + Modeset Base Object Abstraction === diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 98bf130feda5..90c55383f1ee 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -541,22 +541,74 @@ struct drm_mode_get_connector { */ #define DRM_MODE_PROP_ATOMIC0x8000 +/** + * struct drm_mode_property_enum - Description for an enum/bitfield entry. + * @value: numeric value for this enum entry. + * @name: symbolic name for this enum entry. + * + * See struct drm_property_enum for details. + */ struct drm_mode_property_enum { __u64 value; char name[DRM_PROP_NAME_LEN]; }; +/** + * struct drm_mode_get_property - Get property metadata. + * + * User-space can perform a GETPROPERTY ioctl to retrieve information about a + * property. The same property may be attached to multiple objects, see + * "Modeset Base Object Abstraction". + * + * The meaning of the @values_ptr field changes depending on the property type. + * See &drm_property.flags for more details. + * + * The @enum_blob_ptr and @count_enum_blobs fields are only meaningful when the + * property has the type &DRM_MODE_PROP_ENUM or &DRM_MODE_PROP_BITMASK. For + * backwards compatibility, the kernel will always set @count_enum_blobs to + * zero when the property has the type &DRM_MODE_PROP_BLOB. User-space must + * ignore these two fields if the property has a different type. + * + * User-space is expected to retrieve values and enums by performing this ioctl + * at least twice: the first time to retrieve the number of elements, the + * second time to retrieve the elements themselves. + * + * To retrieve the number of elements, set @count_values and @count_enum_blobs + * to zero, then call the ioctl. @count_values will be updated with the number + * of elements. If the property has the type &DRM_MODE_PROP_ENUM or + * &DRM_MODE_PROP_BITMASK, @count_enum_blobs will be updated as well. + * + * To retrieve the elements themselves, allocate an array for @values_ptr and + * set @count_values to its capacity. If the property has the type + * &DRM_MODE_PROP_ENUM or &DRM_MODE_PROP_BITMASK, allocate an array for + * @enum_blob_ptr and set @count_enum_blobs to its capacity. Calling the ioctl + * again will fill the arrays. + */ struct drm_mode_get_property { - __u64 values_ptr; /* values and blob lengths */ - __u64 enum_blob_ptr; /* enum and blob id ptrs */ + /** @values_ptr: Pointer to a ``__u64`` array. */ + __u64 values_ptr; + /** @enum_blob_ptr: Pointer to a struct drm_mode_property_enum array. */ + __u64 enum_blob_ptr; + /** +* @prop_id: Object ID of the property which should be retrieved. Set +* by the caller. +*/ __u32 prop_id; + /** +* @flags: ``DRM_MODE_PROP_*`` bitfield. See &drm_property.flags for +* a definition of the flags. +*/ __u32 flags; + /** +* @name: Symbolic property name. User-space should use this field to +* recognize properties. +*/ char name[DRM_PROP_NAME_LEN]; + /** @count_values: Number of elements in @values_ptr. */ __u32 count_values; - /* This is only used to count enum values, not blobs. The _blobs is -* simply because of a historical reason, i.e. backwards compat. */ + /** @count_enum_blobs: Number of elements in @enum_blob_ptr. */ __u32 count_enum_blobs; }; -- 2.32.0
[PATCH 3/4] drm/tiny: add simple-dbi driver
Add a driver for generic MIPI DBI panels initialized with MIPI DCS commands. Currently a ST7789V-based panel is added to it. This panel has its configuration pre-programmed into the controller, so no vendor-specific configuration is needed. Signed-off-by: Icenowy Zheng --- drivers/gpu/drm/tiny/Kconfig | 12 ++ drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/simple-dbi.c | 244 ++ 3 files changed, 257 insertions(+) create mode 100644 drivers/gpu/drm/tiny/simple-dbi.c diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index d31be274a2bd..6cfc57b68a46 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -144,6 +144,18 @@ config TINYDRM_REPAPER If M is selected the module will be called repaper. +config TINYDRM_SIMPLE_DBI + tristate "DRM support for generic MIPI DBI panels" + depends on DRM && SPI + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DRM_MIPI_DBI + help + DRM driver for generic DBI panels that are MIPI DCS compatible + and needs no vendor-specific setup commands. + + If M is selected the module will be called simple-dbi. + config TINYDRM_ST7586 tristate "DRM support for Sitronix ST7586 display panels" depends on DRM && SPI diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index e09942895c77..2553de651aa3 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -13,3 +13,4 @@ obj-$(CONFIG_TINYDRM_MI0283QT)+= mi0283qt.o obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o obj-$(CONFIG_TINYDRM_ST7586) += st7586.o obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o +obj-$(CONFIG_TINYDRM_SIMPLE_DBI) += simple-dbi.o diff --git a/drivers/gpu/drm/tiny/simple-dbi.c b/drivers/gpu/drm/tiny/simple-dbi.c new file mode 100644 index ..2b207e43d500 --- /dev/null +++ b/drivers/gpu/drm/tiny/simple-dbi.c @@ -0,0 +1,244 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * DRM driver for display panels with configuration preset and needs only + * standard MIPI DCS commands to bring up. + * + * Copyright (C) 2021 Sipeed + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#define MIPI_DCS_ADDRESS_MODE_BGR BIT(3) +#define MIPI_DCS_ADDRESS_MODE_REVERSE BIT(5) +#define MIPI_DCS_ADDRESS_MODE_RTL BIT(6) +#define MIPI_DCS_ADDRESS_MODE_BTT BIT(7) + +struct simple_dbi_cfg { + const struct drm_display_mode mode; + unsigned int left_offset; + unsigned int top_offset; + bool inverted; + bool write_only; + bool bgr; + bool right_to_left; + bool bottom_to_top; +}; + +struct simple_dbi_priv { + struct mipi_dbi_dev dbidev; /* Must be first for .release() */ + const struct simple_dbi_cfg *cfg; +}; + +static void simple_dbi_pipe_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) +{ + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev); + struct simple_dbi_priv *priv = container_of(dbidev, + struct simple_dbi_priv, + dbidev); + struct mipi_dbi *dbi = &dbidev->dbi; + int ret, idx; + u8 addr_mode = 0x00; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return; + + DRM_DEBUG_KMS("\n"); + + ret = mipi_dbi_poweron_reset(dbidev); + if (ret) + goto out_exit; + + mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE); + msleep(5); + + /* Currently tinydrm supports 16bit only now */ + mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, +MIPI_DCS_PIXEL_FMT_16BIT); + + if (priv->cfg->inverted) + mipi_dbi_command(dbi, MIPI_DCS_ENTER_INVERT_MODE); + else + mipi_dbi_command(dbi, MIPI_DCS_EXIT_INVERT_MODE); + + if (priv->cfg->bgr) + addr_mode |= MIPI_DCS_ADDRESS_MODE_BGR; + + if (priv->cfg->right_to_left) + addr_mode |= MIPI_DCS_ADDRESS_MODE_RTL; + + if (priv->cfg->bottom_to_top) + addr_mode |= MIPI_DCS_ADDRESS_MODE_BTT; + + switch (dbidev->rotation) { + default: + break; + case 90: + addr_mode ^= MIPI_DCS_ADDRESS_MODE_REVERSE; + addr_mode ^= MIPI_DCS_ADDRESS_MODE_RTL; + break; + case 180: + addr_mode ^= MIPI_DCS_ADDRESS_MODE_RTL; + addr_mode ^= MIPI_DCS_ADDRESS_MODE_BTT; + break; + case 270: + addr_mode ^= MIPI_DCS_ADDRESS_MODE_REV
[PATCH 0/4] Add simple-dbi tinydrm driver
This patchset adds a tinydrm driver called simple-dbi, which is a driver that utilizes only standardized commands in MIPI DCS to activate a MIPI DBI panel that requires no extra configuration, usually because the configuration is pre-programmed into the OTP of the LCD controller. Icenowy Zheng (4): dt-bindings: vendor-prefixes: add Zhishengxin dt-bindings: display: add binding for simple-dbi panels drm/tiny: add simple-dbi driver MAINTAINERS: add simple-dbi driver .../bindings/display/simple-dbi.yaml | 72 ++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + MAINTAINERS | 7 + drivers/gpu/drm/tiny/Kconfig | 12 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/simple-dbi.c | 244 ++ 6 files changed, 338 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/simple-dbi.yaml create mode 100644 drivers/gpu/drm/tiny/simple-dbi.c -- 2.30.2
[PATCH] gpu/drm/i915: Remove duplicated include of intel_region_lmem.h
Duplicate include header file "intel_region_lmem.h" line 8: #include "intel_region_lmem.h" line 12: #include "intel_region_lmem.h" Signed-off-by: zhouchuangao --- drivers/gpu/drm/i915/gt/intel_region_lmem.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c index f7366b0..119eeec 100644 --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c @@ -9,7 +9,6 @@ #include "intel_region_ttm.h" #include "gem/i915_gem_lmem.h" #include "gem/i915_gem_region.h" -#include "intel_region_lmem.h" static int init_fake_lmem_bar(struct intel_memory_region *mem) { -- 2.7.4
[PATCH 1/4] dt-bindings: vendor-prefixes: add Zhishengxin
Shenzhen Zhishengxin Technology Co., Ltd. is a LCD module supplier. Add vendor prefix for it. Signed-off-by: Icenowy Zheng --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 62cb1d9341f5..d8fdec851f38 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -1334,6 +1334,8 @@ patternProperties: description: Zinitix Co., Ltd "^zkmagic,.*": description: Shenzhen Zkmagic Technology Co., Ltd. + "^zsx,.*": +description: Shenzhen Zhishengxin Technology Co., Ltd. "^zte,.*": description: ZTE Corp. "^zyxel,.*": -- 2.30.2
[PATCH 2/4] dt-bindings: display: add binding for simple-dbi panels
As we're going to introduce a driver for MIPI DBI panels that need only standard MIPI-DCS commands to initialize (usually because the controller has some configuration pre-programmed), add a DT binding file for it, which now includes only one DBI Type C Option 3 panel, ZSX154-B1206. Signed-off-by: Icenowy Zheng --- .../bindings/display/simple-dbi.yaml | 72 +++ 1 file changed, 72 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/simple-dbi.yaml diff --git a/Documentation/devicetree/bindings/display/simple-dbi.yaml b/Documentation/devicetree/bindings/display/simple-dbi.yaml new file mode 100644 index ..f49b0fda0935 --- /dev/null +++ b/Documentation/devicetree/bindings/display/simple-dbi.yaml @@ -0,0 +1,72 @@ +# SPDX-License-Identifier: GPL-2.0-only +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/simple-dbi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Generic MIPI DCS-compatible DBI panels Device Tree Bindings + +maintainers: + - Icenowy Zheng + +description: + This binding is for display panels that utilizes MIPI DBI Type C, follows + MIPI DCS and needs no vendor-specific initialization commands. + +properties: + compatible: +oneOf: + - description: + Zhishengxin ZSX154-B1206 1.54" 240x240 SPI LCD +items: + - const: zsx,zsx154-b1206 + + spi-max-frequency: +maximum: 3000 + + dc-gpios: +maxItems: 1 +description: Display data/command selection (D/CX) + + backlight: true + reg: true + reset-gpios: true + rotation: true + +required: + - compatible + - reg + +allOf: + - $ref: panel/panel-common.yaml# + - if: + properties: +compatible: + contains: +const: zsx,zsx154-b1206 + +then: + required: +- dc-gpios +- reset-gpios + +additionalProperties: false + +examples: + - | +#include + +spi { +#address-cells = <1>; +#size-cells = <0>; + +display@0 { +compatible = "zsx,zsx154-b1206"; +reg = <0>; +spi-max-frequency = <3000>; +dc-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; +reset-gpios = <&r_pio 2 21 GPIO_ACTIVE_HIGH>; +}; +}; + +... -- 2.30.2
[PATCH 4/4] MAINTAINERS: add simple-dbi driver
As I pushed the simple-dbi driver, add myself as the maintainer now. Signed-off-by: Icenowy Zheng --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3a00771b9fe2..e05c4910c062 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5803,6 +5803,13 @@ S: Maintained F: Documentation/devicetree/bindings/display/panel/feiyang,fy07024di26a30d.yaml F: drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c +DRM DRIVER FOR GENERIC MIPI-DBI LCD PANELS +M: Icenowy Zheng +S: Maintained +T: git git://anongit.freedesktop.org/drm/drm-misc +F: Documentation/devicetree/bindings/display/simple-dbi.yaml +F: drivers/gpu/drm/tiny/simple-dbi.c + DRM DRIVER FOR GENERIC USB DISPLAY M: Noralf Trønnes S: Maintained -- 2.30.2
[PATCH] gpu/drm/amd: Remove duplicated include of drm_drv.h
Duplicate include header file line 28: #include line 44: #include Signed-off-by: zhouchuangao --- drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c index 3ec5099..05f864f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c @@ -41,8 +41,6 @@ #include "amdgpu_securedisplay.h" #include "amdgpu_atomfirmware.h" -#include - static int psp_sysfs_init(struct amdgpu_device *adev); static void psp_sysfs_fini(struct amdgpu_device *adev); -- 2.7.4
Re: [PATCH v2 7/7] drm/connector: add ref to drm_connector_get in iter docs
Pushed this one doc patch with Daniel's R-b on IRC.
[PATCH v6 0/3] support gce on mt8192 platform
Change since v5: -rebase on linux 5.14-rc1 Yongqiang Niu (3): dt-binding: gce: add gce header file for mt8192 arm64: dts: mt8192: add gce node mailbox: cmdq: add mt8192 support .../devicetree/bindings/mailbox/mtk-gce.txt| 7 +- arch/arm64/boot/dts/mediatek/mt8192.dtsi | 10 + drivers/mailbox/mtk-cmdq-mailbox.c | 10 + include/dt-bindings/gce/mt8192-gce.h | 335 + 4 files changed, 359 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/gce/mt8192-gce.h -- 1.8.1.1.dirty
[PATCH v6, 1/3] dt-binding: gce: add gce header file for mt8192
Add documentation for the mt8192 gce. Add gce header file defined the gce hardware event, subsys number and constant for mt8192. Signed-off-by: Yongqiang Niu Reviewed-by: Rob Herring Signed-off-by: Hsin-Yi Wang --- .../devicetree/bindings/mailbox/mtk-gce.txt| 7 +- include/dt-bindings/gce/mt8192-gce.h | 335 + 2 files changed, 339 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/gce/mt8192-gce.h diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt index 7771eca..ac424505 100644 --- a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt +++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt @@ -9,8 +9,8 @@ CMDQ driver uses mailbox framework for communication. Please refer to mailbox.txt for generic information about mailbox device-tree bindings. Required properties: -- compatible: can be "mediatek,mt8173-gce", "mediatek,mt8183-gce" or - "mediatek,mt6779-gce". +- compatible: can be "mediatek,mt8173-gce", "mediatek,mt8183-gce", + "mediatek,mt8192-gce" or "mediatek,mt6779-gce". - reg: Address range of the GCE unit - interrupts: The interrupt signal from the GCE block - clock: Clocks according to the common clock binding @@ -36,7 +36,8 @@ Optional properties for a client device: size: the total size of register address that GCE can access. Some vaules of properties are defined in 'dt-bindings/gce/mt8173-gce.h', -'dt-binding/gce/mt8183-gce.h' or 'dt-bindings/gce/mt6779-gce.h'. Such as +'dt-binding/gce/mt8183-gce.h', 'dt-binding/gce/mt8192-gce.h' or +'dt-bindings/gce/mt6779-gce.h'. Such as sub-system ids, thread priority, event ids. Example: diff --git a/include/dt-bindings/gce/mt8192-gce.h b/include/dt-bindings/gce/mt8192-gce.h new file mode 100644 index 000..9e5a0eb --- /dev/null +++ b/include/dt-bindings/gce/mt8192-gce.h @@ -0,0 +1,335 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2020 MediaTek Inc. + * Author: Yongqiang Niu + */ + +#ifndef _DT_BINDINGS_GCE_MT8192_H +#define _DT_BINDINGS_GCE_MT8192_H + +/* assign timeout 0 also means default */ +#define CMDQ_NO_TIMEOUT0x +#define CMDQ_TIMEOUT_DEFAULT 1000 + +/* GCE thread priority */ +#define CMDQ_THR_PRIO_LOWEST 0 +#define CMDQ_THR_PRIO_11 +#define CMDQ_THR_PRIO_22 +#define CMDQ_THR_PRIO_33 +#define CMDQ_THR_PRIO_44 +#define CMDQ_THR_PRIO_55 +#define CMDQ_THR_PRIO_66 +#define CMDQ_THR_PRIO_HIGHEST 7 + +/* CPR count in 32bit register */ +#define GCE_CPR_COUNT 1312 + +/* GCE subsys table */ +#define SUBSYS_13000 +#define SUBSYS_14001 +#define SUBSYS_14012 +#define SUBSYS_14023 +#define SUBSYS_15024 +#define SUBSYS_18805 +#define SUBSYS_18816 +#define SUBSYS_18827 +#define SUBSYS_18838 +#define SUBSYS_18849 +#define SUBSYS_100010 +#define SUBSYS_100111 +#define SUBSYS_100212 +#define SUBSYS_100313 +#define SUBSYS_100414 +#define SUBSYS_100515 +#define SUBSYS_102016 +#define SUBSYS_102817 +#define SUBSYS_170018 +#define SUBSYS_170119 +#define SUBSYS_170220 +#define SUBSYS_170321 +#define SUBSYS_180022 +#define SUBSYS_180123 +#define SUBSYS_180224 +#define SUBSYS_180425 +#define SUBSYS_180526 +#define SUBSYS_180827 +#define SUBSYS_180a28 +#define SUBSYS_180b29 + +#define CMDQ_EVENT_VDEC_LAT_SOF_0 0 +#define CMDQ_EVENT_VDEC_LAT_FRAME_DONE_0 1 +#define CMDQ_EVENT_VDEC_LAT_FRAME_DONE_1 2 +#define CMDQ_EVENT_VDEC_LAT_FRAME_DONE_2 3 +#define CMDQ_EVENT_VDEC_LAT_FRAME_DONE_3 4 +#define CMDQ_EVENT_VDEC_LAT_FRAME_DONE_4 5 +#define CMDQ_EVENT_VDEC_LAT_FRAME_DONE_5 6 +#define CMDQ_EVENT_VDEC_LAT_FRAME_DONE_6 7 +#define CMDQ_EVENT_VDEC_LAT_ENG_EVENT_08 +#define CMDQ_EVENT_VDEC_LAT_ENG_EVENT_19 +#define CMDQ_EVENT_VDEC_LAT_ENG_EVENT_210 +#define CMDQ_EVENT_VDEC_LAT_ENG_EVENT_311 +#define CMDQ_EVENT_VDEC_LAT_ENG_EVENT_412 +#define CMDQ_EVENT_VDEC_LAT_ENG_EVENT_513 +#define CMDQ_EVENT_VDEC_LAT_ENG_EVENT_614 +#define CMDQ_EVENT_VDEC_LAT_ENG_EVENT_715 + +#define CM
[PATCH v6, 2/3] arm64: dts: mt8192: add gce node
add gce node for mt8192 Signed-off-by: Yongqiang Niu Signed-off-by: Hsin-Yi Wang --- arch/arm64/boot/dts/mediatek/mt8192.dtsi | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi index 6b22441..c8472c6 100644 --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi @@ -6,6 +6,7 @@ /dts-v1/; #include +#include #include #include #include @@ -517,6 +518,15 @@ clock-names = "clk13m"; }; + gce: mailbox@10228000 { + compatible = "mediatek,mt8192-gce"; + reg = <0 0x10228000 0 0x4000>; + interrupts = ; + #mbox-cells = <3>; + clocks = <&infracfg CLK_INFRA_GCE>; + clock-names = "gce"; + }; + scp_adsp: clock-controller@1072 { compatible = "mediatek,mt8192-scp_adsp"; reg = <0 0x1072 0 0x1000>; -- 1.8.1.1.dirty
[PATCH v6, 3/3] mailbox: cmdq: add mt8192 support
add mt8192 support Signed-off-by: Yongqiang Niu Signed-off-by: Hsin-Yi Wang --- drivers/mailbox/mtk-cmdq-mailbox.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c index 67a42b5..8d39b98 100644 --- a/drivers/mailbox/mtk-cmdq-mailbox.c +++ b/drivers/mailbox/mtk-cmdq-mailbox.c @@ -36,6 +36,8 @@ #define CMDQ_THR_WAIT_TOKEN0x30 #define CMDQ_THR_PRIORITY 0x40 +#define GCE_GCTL_VALUE 0x48 + #define CMDQ_THR_ACTIVE_SLOT_CYCLES0x3200 #define CMDQ_THR_ENABLED 0x1 #define CMDQ_THR_DISABLED 0x0 @@ -76,11 +78,13 @@ struct cmdq { struct clk *clock; boolsuspended; u8 shift_pa; + boolcontrol_by_sw; }; struct gce_plat { u32 thread_nr; u8 shift; + bool control_by_sw; }; u8 cmdq_get_shift_pa(struct mbox_chan *chan) @@ -121,6 +125,8 @@ static void cmdq_init(struct cmdq *cmdq) int i; WARN_ON(clk_enable(cmdq->clock) < 0); + if (cmdq->control_by_sw) + writel(0x7, cmdq->base + GCE_GCTL_VALUE); writel(CMDQ_THR_ACTIVE_SLOT_CYCLES, cmdq->base + CMDQ_THR_SLOT_CYCLES); for (i = 0; i <= CMDQ_MAX_EVENT; i++) writel(i, cmdq->base + CMDQ_SYNC_TOKEN_UPDATE); @@ -540,6 +546,7 @@ static int cmdq_probe(struct platform_device *pdev) cmdq->thread_nr = plat_data->thread_nr; cmdq->shift_pa = plat_data->shift; + cmdq->control_by_sw = plat_data->control_by_sw; cmdq->irq_mask = GENMASK(cmdq->thread_nr - 1, 0); err = devm_request_irq(dev, cmdq->irq, cmdq_irq_handler, IRQF_SHARED, "mtk_cmdq", cmdq); @@ -605,11 +612,14 @@ static int cmdq_probe(struct platform_device *pdev) static const struct gce_plat gce_plat_v2 = {.thread_nr = 16}; static const struct gce_plat gce_plat_v3 = {.thread_nr = 24}; static const struct gce_plat gce_plat_v4 = {.thread_nr = 24, .shift = 3}; +static const struct gce_plat gce_plat_v5 = {.thread_nr = 24, .shift = 3, + .control_by_sw = true}; static const struct of_device_id cmdq_of_ids[] = { {.compatible = "mediatek,mt8173-gce", .data = (void *)&gce_plat_v2}, {.compatible = "mediatek,mt8183-gce", .data = (void *)&gce_plat_v3}, {.compatible = "mediatek,mt6779-gce", .data = (void *)&gce_plat_v4}, + {.compatible = "mediatek,mt8192-gce", .data = (void *)&gce_plat_v5}, {} }; -- 1.8.1.1.dirty
[PATCH v3] soc: mediatek: cmdq: add address shift in jump
Add address shift when compose jump instruction to compatible with 35bit format. Fixes: 0858fde496f8 ("mailbox: cmdq: variablize address shift in platform") Signed-off-by: Yongqiang Niu Reviewed-by: Nicolas Boichat --- drivers/mailbox/mtk-cmdq-mailbox.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c index 8d39b98..d03bfaf 100644 --- a/drivers/mailbox/mtk-cmdq-mailbox.c +++ b/drivers/mailbox/mtk-cmdq-mailbox.c @@ -174,7 +174,8 @@ static void cmdq_task_insert_into_thread(struct cmdq_task *task) dma_sync_single_for_cpu(dev, prev_task->pa_base, prev_task->pkt->cmd_buf_size, DMA_TO_DEVICE); prev_task_base[CMDQ_NUM_CMD(prev_task->pkt) - 1] = - (u64)CMDQ_JUMP_BY_PA << 32 | task->pa_base; + (u64)CMDQ_JUMP_BY_PA << 32 | + (task->pa_base >> task->cmdq->shift_pa); dma_sync_single_for_device(dev, prev_task->pa_base, prev_task->pkt->cmd_buf_size, DMA_TO_DEVICE); -- 1.8.1.1.dirty
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
On Fri, Jul 30, 2021 at 02:50:10PM +0200, Michel Dänzer wrote: > On 2021-07-30 12:25 p.m., Daniel Vetter wrote: > > On Thu, Jul 29, 2021 at 01:16:55AM -0700, Vivek Kasireddy wrote: > >> By separating the OUT_FENCE signalling from pageflip completion allows > >> a Guest compositor to start a new repaint cycle with a new buffer > >> instead of waiting for the old buffer to be free. > >> > >> This work is based on the idea/suggestion from Simon and Pekka. > >> > >> This capability can be a solution for this issue: > >> https://gitlab.freedesktop.org/wayland/weston/-/issues/514 > >> > >> Corresponding Weston MR: > >> https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/668 > > > > Uh I kinda wanted to discuss this a bit more before we jump into typing > > code, but well I guess not that much work yet. > > > > So maybe I'm not understanding the problem, but I think the fundamental > > underlying issue is that with KMS you can have at most 2 buffers > > in-flight, due to our queue depth limit of 1 pending flip. > > > > Unfortunately that means for virtual hw where it takes a few more > > steps/vblanks until the framebuffer actually shows up on screen and is > > scanned out, we suffer deeply. The usual fix for that is to drop the > > latency and increase throughput, and have more buffers in-flight. Which > > this patch tries to do. > > Per > https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_986797 , > IMO the underlying issue is actually that the guest compositor repaint > cycle is not aligned with the host compositor one. If they were aligned, > the problem would not occur even without allowing multiple page flips in > flight, and latency would be lower. Yeah my proposal here is under the premise that we do actually need to fix this with a deeper queue depth. > > Now I think where we go wrong here is that we're trying to hack this up by > > defining different semantics for the out-fence and for the drm-event. Imo > > that's wrong, they're both meant to show eactly the same thing: > > - when is the new frame actually visible to the user (as in, eyeballs in a > > human head, preferrably, not the time when we've handed the buffer off > > to the virtual hw) > > - when is the previous buffer no longer being used by the scanout hw > > > > We do cheat a bit right now in so far that we assume they're both the > > same, as in, panel-side latency is currently the compositor's problem to > > figure out. > > > > So for virtual hw I think the timestamp and even completion really need to > > happen only when the buffer has been pushed through the entire > > virtualization chain, i.e. ideally we get the timestamp from the kms > > driver from the host side. Currently that's not done, so this is most > > likely quite broken already (virtio relies on the no-vblank auto event > > sending, which definitely doesn't wait for anything, or I'm completely > > missing something). > > > > I think instead of hacking up some ill-defined 1.5 queue depth support, > > what we should do is support queue depth > 1 properly. So: > > > > - Change atomic to support queue depth > 1, this needs to be a per-driver > > thing due to a bunch of issues in driver code. Essentially drivers must > > never look at obj->state pointers, and only ever look up state through > > the passed-in drm_atomic_state * update container. > > > > - Aside: virtio should loose all it's empty hooks, there's no point in > > that. > > > > - We fix virtio to send out the completion event at the end of this entire > > pipeline, i.e. virtio code needs to take care of sending out the > > crtc_state->event correctly. > > > > - We probably also want some kind of (maybe per-crtc) recommended queue > > depth property so compositors know how many buffers to keep in flight. > > Not sure about that. > > I'd say there would definitely need to be some kind of signal for the > display server that it should queue multiple flips, since this is > normally not desirable for latency. In other words, this wouldn't really > be useful on bare metal (in contrast to the ability to replace a pending > flip with a newer one). Hm I was thinking that the compositor can tune this. If the round-trip latency (as measured by events) is too long to get full refresh rate, it can add more buffers to the queue. That's kinda why I think the returned event really must be accurate wrt actual display time (and old buffer release time), so that this computation in the compositor because a pretty simple num_buffers = (flip_time - submit_time) / frame_time With maybe some rounding up and averaging. You can also hit this when your 3d engine has an extremely deep pipeline (like some of the tiling renders have), where rendering just takes forever, but as long as you keep 2 frames in the renderer in-flight you can achieve full refresh rate (at a latency cost). So kernel can't really tell you in all cases how many buffers you should have. -Daniel > -- > Earthling
Re: [PATCH] drm/radeon: Update pitch for page flip
Am 02.08.21 um 09:43 schrieb Zhenneng Li: When primary bo is updated, crtc's pitch may have not been updated, this will lead to show disorder content when user changes display mode, we update crtc's pitch in page flip to avoid this bug. This refers to amdgpu's pageflip. Alex is the expert to ask about that code, but I'm not sure if that is really correct for the old hardware. As far as I know the crtc's pitch should not change during a page flip, but only during a full mode set. So could you elaborate a bit more how you trigger this? Thanks, Christian. Cc: Alex Deucher Cc: "Christian König" Cc: "Pan, Xinhui" Cc: David Airlie Cc: Daniel Vetter Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Zhenneng Li --- drivers/gpu/drm/radeon/evergreen.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 36a888e1b179..eeb590d2dec2 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -28,6 +28,7 @@ #include #include +#include #include "atom.h" #include "avivod.h" @@ -1414,10 +1415,15 @@ void evergreen_page_flip(struct radeon_device *rdev, int crtc_id, u64 crtc_base, bool async) { struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id]; + struct drm_framebuffer *fb = radeon_crtc->base.primary->fb; - /* update the scanout addresses */ + /* flip at hsync for async, default is vsync */ WREG32(EVERGREEN_GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset, async ? EVERGREEN_GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0); + /* update pitch */ + WREG32(EVERGREEN_GRPH_PITCH + radeon_crtc->crtc_offset, + fb->pitches[0] / fb->format->cpp[0]); + /* update the scanout addresses */ WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + radeon_crtc->crtc_offset, upper_32_bits(crtc_base)); WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + radeon_crtc->crtc_offset, No virus found Checked by Hillstone Network AntiVirus
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
On Mon, Aug 02, 2021 at 06:51:33AM +, Kasireddy, Vivek wrote: > Hi Daniel, > > > > > On Thu, Jul 29, 2021 at 01:16:55AM -0700, Vivek Kasireddy wrote: > > > By separating the OUT_FENCE signalling from pageflip completion allows > > > a Guest compositor to start a new repaint cycle with a new buffer > > > instead of waiting for the old buffer to be free. > > > > > > This work is based on the idea/suggestion from Simon and Pekka. > > > > > > This capability can be a solution for this issue: > > > https://gitlab.freedesktop.org/wayland/weston/-/issues/514 > > > > > > Corresponding Weston MR: > > > https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/668 > > > > Uh I kinda wanted to discuss this a bit more before we jump into typing > > code, but well I guess not that much work yet. > [Kasireddy, Vivek] Right, it wasn't a lot of work :) > > > > > So maybe I'm not understanding the problem, but I think the fundamental > > underlying issue is that with KMS you can have at most 2 buffers > > in-flight, due to our queue depth limit of 1 pending flip. > [Kasireddy, Vivek] Let me summarize the problem again from the perspective of > both the Host (Weston) and Guest (Weston) compositors assuming a refresh-rate > of 60 -- which implies the Vblank/Vsync is generated every ~16.66 ms. > Host compositor: > - After a pageflip completion event, it starts its next repaint cycle by > waiting for 9 ms > and then submits the atomic commit and at the tail end of its cycle sends a > frame callback > event to all its clients (who registered and submitted frames) indicating to > them to > start their next redraw -- giving them at-least ~16 ms to submit a new frame > to be > included in its next repaint. Why a configurable 9 ms delay is needed is > explained > in Pekka's blog post here: > https://ppaalanen.blogspot.com/2015/02/weston-repaint-scheduling.html > > - It'll send a wl_buffer.release event for a client submitted previous buffer > only > when the client has submitted a new buffer and: > a) When it hasn't started its repaint cycle yet OR > b) When it clears its old state after it gets a pageflip completion event -- > if it had > flipped the client's buffer onto a hardware plane. > > Guest compositor: > - After a pageflip completion is sent by Guest KMS, it takes about 10-12 ms > for the > Guest compositor to submit a new atomic commit. This time of 10-12 ms > includes the > 9 ms wait -- just like the Host compositor -- for its clients to submit new > buffers. > - When it gets a pageflip completion, it assumes that the previously > submitted buffer > is free for re-use and uses it again -- resulting in the usage of only 2 out > of a maximum > of 4 backbuffers included as part of the Mesa GBM surface implementation. > > Guest KMS/Virtio-gpu/Qemu Wayland UI: > - Because no_vblank=true for Guest KMS and since the vblank event (which also > serves > as the pageflip completion event for user-space) is sent right away after > atomic commit, > as Gerd said, we use an internal dma-fence to block/wait the Guest KMS until > we know for > sure that the Host is completely done using the buffer. To ensure this, we > signal the dma-fence > only after the Host compositor sends a wl_buffer.release event or an > equivalent signal. > > The goal: > - Maintain full framerate even when the Guest scanout FB is flipped onto a > hardware plane > on the Host -- regardless of either compositor's scheduling policy -- without > making any > copies and ensuring that both Host and Guest are not accessing the buffer at > the same time. > > The problem: > - If the Host compositor flips the client's buffer (in this case Guest > compositor's buffer) > onto a hardware plane, then it can send a wl_buffer.release event for the > previous buffer > only after it gets a pageflip completion. And, if the Guest compositor takes > 10-12 ms to > submit a new buffer and given the fact that the Host compositor waits only > for 9 ms, the > Guest compositor will miss the Host's repaint cycle resulting in halved > frame-rate. > > The solution: > - To ensure full framerate, the Guest compositor has to start it's repaint > cycle (including > the 9 ms wait) when the Host compositor sends the frame callback event to its > clients. > In order for this to happen, the dma-fence that the Guest KMS waits on -- > before sending > pageflip completion -- cannot be tied to a wl_buffer.release event. This > means that, the > Guest compositor has to be forced to use a new buffer for its next repaint > cycle when it > gets a pageflip completion. Is that really the only solution? If we fix the event timestamps so that both guest and host use the same timestamp, but then the guest starts 5ms (or something like that) earlier, then things should work too? I.e. - host compositor starts at (previous_frametime + 9ms) - guest compositor starts at (previous_frametime + 4ms) Ofc this only works if the frametimes we hand out to both match _e
Re: [PATCH v2] drm: Fix typo in comments
On Fri, Jul 30, 2021 at 09:27:29PM +0800, Cai Huoqing wrote: > fix typo for drm > > v1->v2: > respin with the change "iff ==> implies that" > > Reviewed-by: Daniel Vetter I did not give you a Reviewed-by: tag, please dont forge them. At least in the linux kernel an r-b tag is a fairly formal statement: https://dri.freedesktop.org/docs/drm/process/submitting-patches.html?highlight=statement%20oversight#reviewer-s-statement-of-oversight The more informal Acked-by is generally ok to add if you get something like "looks good to me", but better to ask for confirmation too in that case. I've removed it before pushing. > Signed-off-by: Cai Huoqing Pushed to drm-misc-next, thanks for your patch. -Daniel > --- > drivers/gpu/drm/drm_aperture.c | 2 +- > drivers/gpu/drm/drm_atomic.c| 2 +- > drivers/gpu/drm/drm_atomic_helper.c | 10 +- > drivers/gpu/drm/drm_atomic_uapi.c | 6 +++--- > drivers/gpu/drm/drm_auth.c | 2 +- > drivers/gpu/drm/drm_bridge.c| 2 +- > drivers/gpu/drm/drm_bufs.c | 2 +- > drivers/gpu/drm/drm_cache.c | 2 +- > drivers/gpu/drm/drm_damage_helper.c | 2 +- > drivers/gpu/drm/drm_dp_helper.c | 8 > drivers/gpu/drm/drm_drv.c | 4 ++-- > drivers/gpu/drm/drm_dsc.c | 2 +- > drivers/gpu/drm/drm_edid.c | 4 ++-- > drivers/gpu/drm/drm_fb_helper.c | 2 +- > drivers/gpu/drm/drm_file.c | 6 +++--- > drivers/gpu/drm/drm_format_helper.c | 2 +- > drivers/gpu/drm/drm_framebuffer.c | 2 +- > drivers/gpu/drm/drm_gem.c | 4 ++-- > drivers/gpu/drm/drm_gem_atomic_helper.c | 4 ++-- > drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- > drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- > drivers/gpu/drm/drm_hdcp.c | 2 +- > drivers/gpu/drm/drm_ioctl.c | 4 ++-- > drivers/gpu/drm/drm_irq.c | 2 +- > drivers/gpu/drm/drm_mm.c| 2 +- > drivers/gpu/drm/drm_mode_object.c | 2 +- > drivers/gpu/drm/drm_modes.c | 4 ++-- > drivers/gpu/drm/drm_plane.c | 2 +- > drivers/gpu/drm/drm_plane_helper.c | 2 +- > drivers/gpu/drm/drm_prime.c | 2 +- > drivers/gpu/drm/drm_probe_helper.c | 2 +- > drivers/gpu/drm/drm_property.c | 2 +- > drivers/gpu/drm/drm_scdc_helper.c | 2 +- > drivers/gpu/drm/drm_syncobj.c | 2 +- > drivers/gpu/drm/drm_vblank.c| 12 ++-- > drivers/gpu/drm/drm_vma_manager.c | 2 +- > 36 files changed, 58 insertions(+), 58 deletions(-) > > diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c > index 9ac39cf11694..74bd4a76b253 100644 > --- a/drivers/gpu/drm/drm_aperture.c > +++ b/drivers/gpu/drm/drm_aperture.c > @@ -78,7 +78,7 @@ > * > * Drivers that are susceptible to being removed by other drivers, such as > * generic EFI or VESA drivers, have to register themselves as owners of > their > - * given framebuffer memory. Ownership of the framebuffer memory is achived > + * given framebuffer memory. Ownership of the framebuffer memory is achieved > * by calling devm_aperture_acquire_from_firmware(). On success, the driver > * is the owner of the framebuffer range. The function fails if the > * framebuffer is already by another driver. See below for an example. > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index d820423fac32..b127e30082ba 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -723,7 +723,7 @@ static void drm_atomic_plane_print_state(struct > drm_printer *p, > * clocks, scaler units, bandwidth and fifo limits shared among a group of > * planes or CRTCs, and so on) it makes sense to model these as independent > * objects. Drivers then need to do similar state tracking and commit > ordering for > - * such private (since not exposed to userpace) objects as the atomic core > and > + * such private (since not exposed to userspace) objects as the atomic core > and > * helpers already provide for connectors, planes and CRTCs. > * > * To make this easier on drivers the atomic core provides some support to > track > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index f7bf1ea62d58..7ee480b6efde 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -634,7 +634,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, >* connectors and a NULL mode. >* >* The other way around is true as well. enable != 0 > - * iff connectors are attached and a mode is set. > + * implies that connectors are attached and a mode is > set. >*/ > new_crtc_state->mode_changed = true; >
Re: [PATCH] drm: property: Replace strncpy() with strscpy_pad()
On Sat, Jul 31, 2021 at 04:32:41AM +0300, Laurent Pinchart wrote: > strncpy() is widely regarded as unsafe due to the fact that it may leave > the destination string without a nul-termination when the source string > size is too large. When compiling the kernel with W=1, the gcc warns > about this: > > drivers/gpu/drm/drm_property.c: In function ‘drm_property_create’: > drivers/gpu/drm/drm_property.c:130:2: warning: ‘strncpy’ specified bound 32 > equals destination size [-Wstringop-truncation] > 130 | strncpy(property->name, name, DRM_PROP_NAME_LEN); > | ^~~~ > > There are three occurrences of strncpy() in drm_property.c. None of them > are actually unsafe, as the very next line forces nul-termination of the > destination buffer. The warning is thus a false positive, but adds noise > to the kernel log. It can easily be silenced by using strscpy_pad() > instead. Do so. > > One of the three occurrences, in drm_property_add_enum(), fills a char > array that is later copied to userspace with copy_to_user() in > drm_mode_getproperty_ioctl(). To avoid leaking kernel data, > strscpy_pad() is required. Similarly, a second occurrence, in > drm_mode_getproperty_ioctl(), copies the string to an ioctl data buffer > that isn't previously zero'ed, to strscpy_pad() is also required. The > last occurrence, in drm_property_create(), would be safe to replace with > strscpy(), as the destination buffer is copied to userspace with > strscpy_pad(). However, given that this isn't in a hot path, let's avoid > future data leaks in case someone copies the whole char array blindly. +1 on just playing it safe. > Signed-off-by: Laurent Pinchart Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_property.c | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c > index 27c824a6eb60..32404891446e 100644 > --- a/drivers/gpu/drm/drm_property.c > +++ b/drivers/gpu/drm/drm_property.c > @@ -127,8 +127,7 @@ struct drm_property *drm_property_create(struct > drm_device *dev, > property->num_values = num_values; > INIT_LIST_HEAD(&property->enum_list); > > - strncpy(property->name, name, DRM_PROP_NAME_LEN); > - property->name[DRM_PROP_NAME_LEN-1] = '\0'; > + strscpy_pad(property->name, name, DRM_PROP_NAME_LEN); > > list_add_tail(&property->head, &dev->mode_config.property_list); > > @@ -421,8 +420,7 @@ int drm_property_add_enum(struct drm_property *property, > if (!prop_enum) > return -ENOMEM; > > - strncpy(prop_enum->name, name, DRM_PROP_NAME_LEN); > - prop_enum->name[DRM_PROP_NAME_LEN-1] = '\0'; > + strscpy_pad(prop_enum->name, name, DRM_PROP_NAME_LEN); > prop_enum->value = value; > > property->values[index] = value; > @@ -475,8 +473,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev, > if (!property) > return -ENOENT; > > - strncpy(out_resp->name, property->name, DRM_PROP_NAME_LEN); > - out_resp->name[DRM_PROP_NAME_LEN-1] = 0; > + strscpy_pad(out_resp->name, property->name, DRM_PROP_NAME_LEN); > out_resp->flags = property->flags; > > value_count = property->num_values; > -- > Regards, > > Laurent Pinchart > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c
On Sat, Jul 31, 2021 at 04:24:56PM +0800, Desmond Cheong Zhi Xi wrote: > Hi, > > Following a discussion on the patch ("drm: use the lookup lock in > drm_is_current_master") [1], Peter Zijlstra proposed new lockdep_assert > helpers to make it convenient to compose lockdep checks together. > > This series includes the patch that introduces the new lockdep helpers, > then utilizes these helpers in drm_is_current_master_locked in the > following patch. > > v1 -> v2: > Patch 2: > - Updated the kerneldoc on the lock design of drm_file.master to explain > the use of lockdep_assert(). As suggested by Boqun Feng. > > Link: > https://lore.kernel.org/lkml/20210722092929.244629-2-desmondcheon...@gmail.com/ > [1] Can you pls also cc: this to intel-gfx so the local CI there can pick it up and verify? Just to check we got it all. -Daniel > > Best wishes, > Desmond > > Desmond Cheong Zhi Xi (1): > drm: add lockdep assert to drm_is_current_master_locked > > Peter Zijlstra (1): > locking/lockdep: Provide lockdep_assert{,_once}() helpers > > drivers/gpu/drm/drm_auth.c | 6 +++--- > include/drm/drm_file.h | 4 > include/linux/lockdep.h| 41 +++--- > 3 files changed, 28 insertions(+), 23 deletions(-) > > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v2] drm: document drm_mode_get_property
On Mon, Aug 02, 2021 at 07:28:35AM +, Simon Ser wrote: > It's not obvious what the fields mean and how they should be used. > The most important detail is the link to drm_property.flags, which > describes how property types work. > > v2: document enum drm_mode_property_enum, add ref to "Modeset Base > Object Abstraction" (Daniel) > > Signed-off-by: Simon Ser > Acked-by: Pekka Paalanen > Cc: Daniel Vetter > Cc: Leandro Ribeiro Acked-by: Daniel Vetter > --- > > I couldn't figure out how to linkify that ref, so it's not linkified. > > Documentation/gpu/drm-kms.rst | 2 ++ > include/uapi/drm/drm_mode.h | 60 --- > 2 files changed, 58 insertions(+), 4 deletions(-) > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > index 0cc21f6aaef5..1ef7951ded5e 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -159,6 +159,8 @@ KMS Core Structures and Functions > .. kernel-doc:: drivers/gpu/drm/drm_mode_config.c > :export: > > +.. _kms_base_object_abstraction: > + > Modeset Base Object Abstraction > === > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 98bf130feda5..90c55383f1ee 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -541,22 +541,74 @@ struct drm_mode_get_connector { > */ > #define DRM_MODE_PROP_ATOMIC0x8000 > > +/** > + * struct drm_mode_property_enum - Description for an enum/bitfield entry. > + * @value: numeric value for this enum entry. > + * @name: symbolic name for this enum entry. > + * > + * See struct drm_property_enum for details. > + */ > struct drm_mode_property_enum { > __u64 value; > char name[DRM_PROP_NAME_LEN]; > }; > > +/** > + * struct drm_mode_get_property - Get property metadata. > + * > + * User-space can perform a GETPROPERTY ioctl to retrieve information about a > + * property. The same property may be attached to multiple objects, see > + * "Modeset Base Object Abstraction". > + * > + * The meaning of the @values_ptr field changes depending on the property > type. > + * See &drm_property.flags for more details. > + * > + * The @enum_blob_ptr and @count_enum_blobs fields are only meaningful when > the > + * property has the type &DRM_MODE_PROP_ENUM or &DRM_MODE_PROP_BITMASK. For > + * backwards compatibility, the kernel will always set @count_enum_blobs to > + * zero when the property has the type &DRM_MODE_PROP_BLOB. User-space must > + * ignore these two fields if the property has a different type. > + * > + * User-space is expected to retrieve values and enums by performing this > ioctl > + * at least twice: the first time to retrieve the number of elements, the > + * second time to retrieve the elements themselves. > + * > + * To retrieve the number of elements, set @count_values and > @count_enum_blobs > + * to zero, then call the ioctl. @count_values will be updated with the > number > + * of elements. If the property has the type &DRM_MODE_PROP_ENUM or > + * &DRM_MODE_PROP_BITMASK, @count_enum_blobs will be updated as well. > + * > + * To retrieve the elements themselves, allocate an array for @values_ptr and > + * set @count_values to its capacity. If the property has the type > + * &DRM_MODE_PROP_ENUM or &DRM_MODE_PROP_BITMASK, allocate an array for > + * @enum_blob_ptr and set @count_enum_blobs to its capacity. Calling the > ioctl > + * again will fill the arrays. > + */ > struct drm_mode_get_property { > - __u64 values_ptr; /* values and blob lengths */ > - __u64 enum_blob_ptr; /* enum and blob id ptrs */ > + /** @values_ptr: Pointer to a ``__u64`` array. */ > + __u64 values_ptr; > + /** @enum_blob_ptr: Pointer to a struct drm_mode_property_enum array. */ > + __u64 enum_blob_ptr; > > + /** > + * @prop_id: Object ID of the property which should be retrieved. Set > + * by the caller. > + */ > __u32 prop_id; > + /** > + * @flags: ``DRM_MODE_PROP_*`` bitfield. See &drm_property.flags for > + * a definition of the flags. > + */ > __u32 flags; > + /** > + * @name: Symbolic property name. User-space should use this field to > + * recognize properties. > + */ > char name[DRM_PROP_NAME_LEN]; > > + /** @count_values: Number of elements in @values_ptr. */ > __u32 count_values; > - /* This is only used to count enum values, not blobs. The _blobs is > - * simply because of a historical reason, i.e. backwards compat. */ > + /** @count_enum_blobs: Number of elements in @enum_blob_ptr. */ > __u32 count_enum_blobs; > }; > > -- > 2.32.0 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] drm/radeon: Update pitch for page flip
On Mon, Aug 02, 2021 at 10:12:47AM +0200, Christian König wrote: > Am 02.08.21 um 09:43 schrieb Zhenneng Li: > > When primary bo is updated, crtc's pitch may > > have not been updated, this will lead to show > > disorder content when user changes display mode, > > we update crtc's pitch in page flip to avoid > > this bug. > > This refers to amdgpu's pageflip. > > Alex is the expert to ask about that code, but I'm not sure if that is > really correct for the old hardware. > > As far as I know the crtc's pitch should not change during a page flip, but > only during a full mode set. > > So could you elaborate a bit more how you trigger this? legacy page_flip ioctl only verifies that the fb->format stays the same. It doesn't check anything else (afair never has), this is all up to drivers to verify. Personally I'd say add a check to reject this, since testing this and making sure it really works everywhere is probably a bit much on this old hw. -Daniel > > Thanks, > Christian. > > > > > Cc: Alex Deucher > > Cc: "Christian König" > > Cc: "Pan, Xinhui" > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: amd-...@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org > > Cc: linux-ker...@vger.kernel.org > > Signed-off-by: Zhenneng Li > > --- > > drivers/gpu/drm/radeon/evergreen.c | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/radeon/evergreen.c > > b/drivers/gpu/drm/radeon/evergreen.c > > index 36a888e1b179..eeb590d2dec2 100644 > > --- a/drivers/gpu/drm/radeon/evergreen.c > > +++ b/drivers/gpu/drm/radeon/evergreen.c > > @@ -28,6 +28,7 @@ > > #include > > #include > > +#include > > #include "atom.h" > > #include "avivod.h" > > @@ -1414,10 +1415,15 @@ void evergreen_page_flip(struct radeon_device > > *rdev, int crtc_id, u64 crtc_base, > > bool async) > > { > > struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id]; > > + struct drm_framebuffer *fb = radeon_crtc->base.primary->fb; > > - /* update the scanout addresses */ > > + /* flip at hsync for async, default is vsync */ > > WREG32(EVERGREEN_GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset, > >async ? EVERGREEN_GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0); > > + /* update pitch */ > > + WREG32(EVERGREEN_GRPH_PITCH + radeon_crtc->crtc_offset, > > + fb->pitches[0] / fb->format->cpp[0]); > > + /* update the scanout addresses */ > > WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + > > radeon_crtc->crtc_offset, > >upper_32_bits(crtc_base)); > > WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + > > radeon_crtc->crtc_offset, > > > > No virus found > > Checked by Hillstone Network AntiVirus > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
RE: [PATCH v2] drm: Fix typo in comments
Hello, > -Original Message- > From: Daniel Vetter > Sent: 2021年8月2日 16:20 > To: Cai,Huoqing > Cc: maarten.lankho...@linux.intel.com; mrip...@kernel.org; > tzimmerm...@suse.de; airl...@linux.ie; dan...@ffwll.ch; dri- > de...@lists.freedesktop.org > Subject: Re: [PATCH v2] drm: Fix typo in comments > > On Fri, Jul 30, 2021 at 09:27:29PM +0800, Cai Huoqing wrote: > > fix typo for drm > > > > v1->v2: > > respin with the change "iff ==> implies that" > > > > Reviewed-by: Daniel Vetter > > I did not give you a Reviewed-by: tag, please dont forge them. At least in > the linux > kernel an r-b tag is a fairly formal statement: > > https://dri.freedesktop.org/docs/drm/process/submitting- > patches.html?highlight=statement%20oversight#reviewer-s-statement-of- > oversight > > The more informal Acked-by is generally ok to add if you get something like > "looks > good to me", but better to ask for confirmation too in that case. I'm very sorry, this is my mistake I will remove it. > > I've removed it before pushing. It's ok > > > Signed-off-by: Cai Huoqing > > Pushed to drm-misc-next, thanks for your patch. > -Daniel > > > --- > > drivers/gpu/drm/drm_aperture.c | 2 +- > > drivers/gpu/drm/drm_atomic.c| 2 +- > > drivers/gpu/drm/drm_atomic_helper.c | 10 +- > > drivers/gpu/drm/drm_atomic_uapi.c | 6 +++--- > > drivers/gpu/drm/drm_auth.c | 2 +- > > drivers/gpu/drm/drm_bridge.c| 2 +- > > drivers/gpu/drm/drm_bufs.c | 2 +- > > drivers/gpu/drm/drm_cache.c | 2 +- > > drivers/gpu/drm/drm_damage_helper.c | 2 +- > > drivers/gpu/drm/drm_dp_helper.c | 8 > > drivers/gpu/drm/drm_drv.c | 4 ++-- > > drivers/gpu/drm/drm_dsc.c | 2 +- > > drivers/gpu/drm/drm_edid.c | 4 ++-- > > drivers/gpu/drm/drm_fb_helper.c | 2 +- > > drivers/gpu/drm/drm_file.c | 6 +++--- > > drivers/gpu/drm/drm_format_helper.c | 2 +- > > drivers/gpu/drm/drm_framebuffer.c | 2 +- > > drivers/gpu/drm/drm_gem.c | 4 ++-- > > drivers/gpu/drm/drm_gem_atomic_helper.c | 4 ++-- > > drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- > > drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- > > drivers/gpu/drm/drm_hdcp.c | 2 +- > > drivers/gpu/drm/drm_ioctl.c | 4 ++-- > > drivers/gpu/drm/drm_irq.c | 2 +- > > drivers/gpu/drm/drm_mm.c| 2 +- > > drivers/gpu/drm/drm_mode_object.c | 2 +- > > drivers/gpu/drm/drm_modes.c | 4 ++-- > > drivers/gpu/drm/drm_plane.c | 2 +- > > drivers/gpu/drm/drm_plane_helper.c | 2 +- > > drivers/gpu/drm/drm_prime.c | 2 +- > > drivers/gpu/drm/drm_probe_helper.c | 2 +- > > drivers/gpu/drm/drm_property.c | 2 +- > > drivers/gpu/drm/drm_scdc_helper.c | 2 +- > > drivers/gpu/drm/drm_syncobj.c | 2 +- > > drivers/gpu/drm/drm_vblank.c| 12 ++-- > > drivers/gpu/drm/drm_vma_manager.c | 2 +- > > 36 files changed, 58 insertions(+), 58 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_aperture.c > > b/drivers/gpu/drm/drm_aperture.c index 9ac39cf11694..74bd4a76b253 > > 100644 > > --- a/drivers/gpu/drm/drm_aperture.c > > +++ b/drivers/gpu/drm/drm_aperture.c > > @@ -78,7 +78,7 @@ > > * > > * Drivers that are susceptible to being removed by other drivers, such as > > * generic EFI or VESA drivers, have to register themselves as owners > > of their > > - * given framebuffer memory. Ownership of the framebuffer memory is > > achived > > + * given framebuffer memory. Ownership of the framebuffer memory is > > + achieved > > * by calling devm_aperture_acquire_from_firmware(). On success, the driver > > * is the owner of the framebuffer range. The function fails if the > > * framebuffer is already by another driver. See below for an example. > > diff --git a/drivers/gpu/drm/drm_atomic.c > > b/drivers/gpu/drm/drm_atomic.c index d820423fac32..b127e30082ba 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -723,7 +723,7 @@ static void drm_atomic_plane_print_state(struct > drm_printer *p, > > * clocks, scaler units, bandwidth and fifo limits shared among a group of > > * planes or CRTCs, and so on) it makes sense to model these as independent > > * objects. Drivers then need to do similar state tracking and commit > > ordering for > > - * such private (since not exposed to userpace) objects as the atomic > > core and > > + * such private (since not exposed to userspace) objects as the > > + atomic core and > > * helpers already provide for connectors, planes and CRTCs. > > * > > * To make this easier on drivers the atomic core provides some > > support to track diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > b/drivers/gpu/drm/drm_atomic_helper.c > > index f7bf1ea62d58..7ee480b6efde 100644
Re: [PATCH 00/14] drm: Make DRM's IRQ helpers legacy
Hi Sam Am 01.08.21 um 22:24 schrieb Sam Ravnborg: Hi Thomas, 1) IRQ_NOTCONNECTED We do not have this check in drm_irq today and we should avoid spreading it all over. We are either carrying it forever or we wil lsee patches floating in to drop the check again. The current use in the kernel is minimal: https://elixir.bootlin.com/linux/latest/A/ident/IRQ_NOTCONNECTED So as a minimum drop it from atmel_hlcdc and preferably from the rest as it is really not used. (Speaking as atmel_hlcdc maintainer) I'll drop it from atmel_hlcdc then. But saying that it's not used is not correct. My point is the drm_irq do not check this - so adding this check add something there was not needed/done before. What is being done at [1] is actually a check for unassigned interrupts. It's just that both, test and errno code, are plain wrong. The patchset fixes this. 2) devm_request_irq() We are moving towards managed allocation so we do not fail to free resources. And an irq has a lifetime equal the device itself - so an obvious cnadidate for devm_request_irq. If we do not introduce it now we will see a revisit of this later. I can be convinced to wait with this as we will have to do much more in each driver, but I cannot see any good arguments to avoid the more modern way to use devm_request_irq. I'll change this in atmel_hdlcd and maybe I can find trivial cases where devm_request_irq() can be used. But drivers that had an uninstall callback before should not have the cleanup logic altered by a patch as this one. I suspect that most of the IRQ cleanup is actually a vblank cleanup and should be done in response to drm_vblank_init(). But that's again not something for this patchset here. We cannot change multiple things at once and still expect any of it to work. I welcome the use of devm_ et al. But these changes are better done in a per-driver patchset that changes all of the driver to managed release. Fair enough, and fine with me. I have yet to read through all patches - will do so in the coming week. OK, thanks a lot. I'll send out a new revision soon, so maybe don't waste time with this one. Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_irq.c#L111 Sam -- 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 OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 01/14] drm/amdgpu: Convert to Linux IRQ interfaces
Hi Am 28.07.21 um 16:03 schrieb Alex Deucher: On Wed, Jul 28, 2021 at 6:27 AM Christian König wrote: Am 27.07.21 um 20:27 schrieb Thomas Zimmermann: Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers don't benefit from using it. DRM IRQ callbacks are now being called directly or inlined. The interrupt number returned by pci_msi_vector() is now stored in struct amdgpu_irq. Calls to pci_msi_vector() can fail and return a negative errno code. Abort initlaizaton in thi case. The DRM IRQ midlayer does not handle this correctly. Signed-off-by: Thomas Zimmermann Alex needs to take a look at this as well, but of hand the patch is Acked-by: Christian König . Looks good to me as well: Reviewed-by: Alex Deucher Thanks a lot. Do you have comments on the changes to radeon? Best regards Thomas Thanks, Christian. --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 21 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 2bd13fc2541a..1e05b5aa94e7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -1775,7 +1775,6 @@ static const struct drm_driver amdgpu_kms_driver = { .open = amdgpu_driver_open_kms, .postclose = amdgpu_driver_postclose_kms, .lastclose = amdgpu_driver_lastclose_kms, - .irq_handler = amdgpu_irq_handler, .ioctls = amdgpu_ioctls_kms, .num_ioctls = ARRAY_SIZE(amdgpu_ioctls_kms), .dumb_create = amdgpu_mode_dumb_create, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c index 0d01cfaca77e..a36cdc7323f4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c @@ -46,7 +46,6 @@ #include #include -#include #include #include #include @@ -184,7 +183,7 @@ void amdgpu_irq_disable_all(struct amdgpu_device *adev) * Returns: * result of handling the IRQ, as defined by &irqreturn_t */ -irqreturn_t amdgpu_irq_handler(int irq, void *arg) +static irqreturn_t amdgpu_irq_handler(int irq, void *arg) { struct drm_device *dev = (struct drm_device *) arg; struct amdgpu_device *adev = drm_to_adev(dev); @@ -307,6 +306,7 @@ static void amdgpu_restore_msix(struct amdgpu_device *adev) int amdgpu_irq_init(struct amdgpu_device *adev) { int r = 0; + unsigned int irq; spin_lock_init(&adev->irq.lock); @@ -349,15 +349,22 @@ int amdgpu_irq_init(struct amdgpu_device *adev) INIT_WORK(&adev->irq.ih2_work, amdgpu_irq_handle_ih2); INIT_WORK(&adev->irq.ih_soft_work, amdgpu_irq_handle_ih_soft); - adev->irq.installed = true; - /* Use vector 0 for MSI-X */ - r = drm_irq_install(adev_to_drm(adev), pci_irq_vector(adev->pdev, 0)); + /* Use vector 0 for MSI-X. */ + r = pci_irq_vector(adev->pdev, 0); + if (r < 0) + return r; + irq = r; + + /* PCI devices require shared interrupts. */ + r = request_irq(irq, amdgpu_irq_handler, IRQF_SHARED, adev_to_drm(adev)->driver->name, + adev_to_drm(adev)); if (r) { - adev->irq.installed = false; if (!amdgpu_device_has_dc_support(adev)) flush_work(&adev->hotplug_work); return r; } + adev->irq.installed = true; + adev->irq.irq = irq; adev_to_drm(adev)->max_vblank_count = 0x00ff; DRM_DEBUG("amdgpu: irq initialized.\n"); @@ -368,7 +375,7 @@ int amdgpu_irq_init(struct amdgpu_device *adev) void amdgpu_irq_fini_hw(struct amdgpu_device *adev) { if (adev->irq.installed) { - drm_irq_uninstall(&adev->ddev); + free_irq(adev->irq.irq, adev_to_drm(adev)); adev->irq.installed = false; if (adev->irq.msi_enabled) pci_free_irq_vectors(adev->pdev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h index 78ad4784cc74..e9f2c11ea416 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h @@ -80,6 +80,7 @@ struct amdgpu_irq_src_funcs { struct amdgpu_irq { boolinstalled; + unsigned intirq; spinlock_t lock; /* interrupt sources */ struct amdgpu_irq_clientclient[AMDGPU_IRQ_CLIENTID_MAX]; @@ -100,7 +101,6 @@ struct amdgpu_irq { }; void amdgpu_irq_disable_all(struct amdgpu_device *adev); -irqreturn_t amdgpu_irq_handler(int irq, void *arg); int amdgpu_irq_init(struct amdgpu_device *adev); void amdgpu_irq_fini_sw(struct amdgpu_device *adev); -- Thomas Zimmermann Graphics Driver Dev
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
On 2021-08-02 9:59 a.m., Daniel Vetter wrote: > On Fri, Jul 30, 2021 at 02:50:10PM +0200, Michel Dänzer wrote: >> On 2021-07-30 12:25 p.m., Daniel Vetter wrote: >>> On Thu, Jul 29, 2021 at 01:16:55AM -0700, Vivek Kasireddy wrote: By separating the OUT_FENCE signalling from pageflip completion allows a Guest compositor to start a new repaint cycle with a new buffer instead of waiting for the old buffer to be free. This work is based on the idea/suggestion from Simon and Pekka. This capability can be a solution for this issue: https://gitlab.freedesktop.org/wayland/weston/-/issues/514 Corresponding Weston MR: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/668 >>> >>> Uh I kinda wanted to discuss this a bit more before we jump into typing >>> code, but well I guess not that much work yet. >>> >>> So maybe I'm not understanding the problem, but I think the fundamental >>> underlying issue is that with KMS you can have at most 2 buffers >>> in-flight, due to our queue depth limit of 1 pending flip. >>> >>> Unfortunately that means for virtual hw where it takes a few more >>> steps/vblanks until the framebuffer actually shows up on screen and is >>> scanned out, we suffer deeply. The usual fix for that is to drop the >>> latency and increase throughput, and have more buffers in-flight. Which >>> this patch tries to do. >> >> Per >> https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_986797 , >> IMO the underlying issue is actually that the guest compositor repaint >> cycle is not aligned with the host compositor one. If they were aligned, >> the problem would not occur even without allowing multiple page flips in >> flight, and latency would be lower. > > Yeah my proposal here is under the premise that we do actually need to fix > this with a deeper queue depth. > >>> Now I think where we go wrong here is that we're trying to hack this up by >>> defining different semantics for the out-fence and for the drm-event. Imo >>> that's wrong, they're both meant to show eactly the same thing: >>> - when is the new frame actually visible to the user (as in, eyeballs in a >>> human head, preferrably, not the time when we've handed the buffer off >>> to the virtual hw) >>> - when is the previous buffer no longer being used by the scanout hw >>> >>> We do cheat a bit right now in so far that we assume they're both the >>> same, as in, panel-side latency is currently the compositor's problem to >>> figure out. >>> >>> So for virtual hw I think the timestamp and even completion really need to >>> happen only when the buffer has been pushed through the entire >>> virtualization chain, i.e. ideally we get the timestamp from the kms >>> driver from the host side. Currently that's not done, so this is most >>> likely quite broken already (virtio relies on the no-vblank auto event >>> sending, which definitely doesn't wait for anything, or I'm completely >>> missing something). >>> >>> I think instead of hacking up some ill-defined 1.5 queue depth support, >>> what we should do is support queue depth > 1 properly. So: >>> >>> - Change atomic to support queue depth > 1, this needs to be a per-driver >>> thing due to a bunch of issues in driver code. Essentially drivers must >>> never look at obj->state pointers, and only ever look up state through >>> the passed-in drm_atomic_state * update container. >>> >>> - Aside: virtio should loose all it's empty hooks, there's no point in >>> that. >>> >>> - We fix virtio to send out the completion event at the end of this entire >>> pipeline, i.e. virtio code needs to take care of sending out the >>> crtc_state->event correctly. >>> >>> - We probably also want some kind of (maybe per-crtc) recommended queue >>> depth property so compositors know how many buffers to keep in flight. >>> Not sure about that. >> >> I'd say there would definitely need to be some kind of signal for the >> display server that it should queue multiple flips, since this is >> normally not desirable for latency. In other words, this wouldn't really >> be useful on bare metal (in contrast to the ability to replace a pending >> flip with a newer one). > > Hm I was thinking that the compositor can tune this. If the round-trip > latency (as measured by events) is too long to get full refresh rate, it > can add more buffers to the queue. That's kinda why I think the returned > event really must be accurate wrt actual display time (and old buffer > release time), so that this computation in the compositor because a pretty > simple > > num_buffers = (flip_time - submit_time) / frame_time > > With maybe some rounding up and averaging. You can also hit this when your > 3d engine has an extremely deep pipeline (like some of the tiling > renders have), where rendering just takes forever, but as long as you keep > 2 frames in the renderer in-flight you can achieve full refresh rate (at a > latency cost). As long as a pag
[PATCH v8, 0/2] soc: mediatek: mmsys: add mt8192 mmsys support
base v5.14-rc1 Yongqiang Niu (2): soc: mediatek: mmsys: add comp OVL_2L2/POSTMASK/RDMA4 soc: mediatek: mmsys: Add mt8192 mmsys routing table drivers/soc/mediatek/mt8192-mmsys.h| 67 ++ drivers/soc/mediatek/mtk-mmsys.c | 11 ++ include/linux/soc/mediatek/mtk-mmsys.h | 3 ++ 3 files changed, 81 insertions(+) create mode 100644 drivers/soc/mediatek/mt8192-mmsys.h -- 1.8.1.1.dirty
[PATCH v8, 2/2] soc: mediatek: mmsys: Add mt8192 mmsys routing table
mt8192 has different routing registers than mt8183 Signed-off-by: Yongqiang Niu --- drivers/soc/mediatek/mt8192-mmsys.h | 67 + drivers/soc/mediatek/mtk-mmsys.c| 11 ++ 2 files changed, 78 insertions(+) create mode 100644 drivers/soc/mediatek/mt8192-mmsys.h diff --git a/drivers/soc/mediatek/mt8192-mmsys.h b/drivers/soc/mediatek/mt8192-mmsys.h new file mode 100644 index 000..0e4b233 --- /dev/null +++ b/drivers/soc/mediatek/mt8192-mmsys.h @@ -0,0 +1,67 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __SOC_MEDIATEK_MT8192_MMSYS_H +#define __SOC_MEDIATEK_MT8192_MMSYS_H + +#define MT8192_MMSYS_OVL_MOUT_EN 0xf04 +#define MT8192_DISP_OVL1_2L_MOUT_EN0xf08 +#define MT8192_DISP_OVL0_2L_MOUT_EN0xf18 +#define MT8192_DISP_OVL0_MOUT_EN 0xf1c +#define MT8192_DISP_RDMA0_SEL_IN 0xf2c +#define MT8192_DISP_RDMA0_SOUT_SEL 0xf30 +#define MT8192_DISP_CCORR0_SOUT_SEL0xf34 +#define MT8192_DISP_AAL0_SEL_IN0xf38 +#define MT8192_DISP_DITHER0_MOUT_EN0xf3c +#define MT8192_DISP_DSI0_SEL_IN0xf40 +#define MT8192_DISP_OVL2_2L_MOUT_EN0xf4c + +#define MT8192_DISP_OVL0_GO_BLEND BIT(0) +#define MT8192_DITHER0_MOUT_IN_DSI0BIT(0) +#define MT8192_OVL0_MOUT_EN_DISP_RDMA0 BIT(0) +#define MT8192_OVL2_2L_MOUT_EN_RDMA4 BIT(0) +#define MT8192_DISP_OVL0_GO_BG BIT(1) +#define MT8192_DISP_OVL0_2L_GO_BLEND BIT(2) +#define MT8192_DISP_OVL0_2L_GO_BG BIT(3) +#define MT8192_OVL1_2L_MOUT_EN_RDMA1 BIT(4) +#define MT8192_OVL0_MOUT_EN_OVL0_2LBIT(4) +#define MT8192_RDMA0_SEL_IN_OVL0_2L0x3 +#define MT8192_RDMA0_SOUT_COLOR0 0x1 +#define MT8192_CCORR0_SOUT_AAL00x1 +#define MT8192_AAL0_SEL_IN_CCORR0 0x1 +#define MT8192_DSI0_SEL_IN_DITHER0 0x1 + +static const struct mtk_mmsys_routes mmsys_mt8192_routing_table[] = { + { + DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0, + MT8192_DISP_OVL0_2L_MOUT_EN, MT8192_OVL0_MOUT_EN_DISP_RDMA0, + }, { + DDP_COMPONENT_OVL_2L2, DDP_COMPONENT_RDMA4, + MT8192_DISP_OVL2_2L_MOUT_EN, MT8192_OVL2_2L_MOUT_EN_RDMA4 + }, { + DDP_COMPONENT_DITHER, DDP_COMPONENT_DSI0, + MT8192_DISP_DITHER0_MOUT_EN, MT8192_DITHER0_MOUT_IN_DSI0 + }, { + DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0, + MT8192_DISP_RDMA0_SEL_IN, MT8192_RDMA0_SEL_IN_OVL0_2L + }, { + DDP_COMPONENT_CCORR, DDP_COMPONENT_AAL0, + MT8192_DISP_AAL0_SEL_IN, MT8192_AAL0_SEL_IN_CCORR0 + }, { + DDP_COMPONENT_DITHER, DDP_COMPONENT_DSI0, + MT8192_DISP_DSI0_SEL_IN, MT8192_DSI0_SEL_IN_DITHER0 + }, { + DDP_COMPONENT_RDMA0, DDP_COMPONENT_COLOR0, + MT8192_DISP_RDMA0_SOUT_SEL, MT8192_RDMA0_SOUT_COLOR0 + }, { + DDP_COMPONENT_CCORR, DDP_COMPONENT_AAL0, + MT8192_DISP_CCORR0_SOUT_SEL, MT8192_CCORR0_SOUT_AAL0 + }, { + DDP_COMPONENT_OVL0, DDP_COMPONENT_OVL_2L0, + MT8192_MMSYS_OVL_MOUT_EN, MT8192_DISP_OVL0_GO_BG, + }, { + DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_RDMA0, + MT8192_MMSYS_OVL_MOUT_EN, MT8192_DISP_OVL0_2L_GO_BLEND, + } +}; + +#endif /* __SOC_MEDIATEK_MT8192_MMSYS_H */ diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c index 080660e..de7b122 100644 --- a/drivers/soc/mediatek/mtk-mmsys.c +++ b/drivers/soc/mediatek/mtk-mmsys.c @@ -13,6 +13,7 @@ #include "mtk-mmsys.h" #include "mt8167-mmsys.h" #include "mt8183-mmsys.h" +#include "mt8192-mmsys.h" static const struct mtk_mmsys_driver_data mt2701_mmsys_driver_data = { .clk_driver = "clk-mt2701-mm", @@ -52,6 +53,12 @@ .num_routes = ARRAY_SIZE(mmsys_mt8183_routing_table), }; +static const struct mtk_mmsys_driver_data mt8192_mmsys_driver_data = { + .clk_driver = "clk-mt8192-mm", + .routes = mmsys_mt8192_routing_table, + .num_routes = ARRAY_SIZE(mmsys_mt8192_routing_table), +}; + struct mtk_mmsys { void __iomem *regs; const struct mtk_mmsys_driver_data *data; @@ -157,6 +164,10 @@ static int mtk_mmsys_probe(struct platform_device *pdev) .compatible = "mediatek,mt8183-mmsys", .data = &mt8183_mmsys_driver_data, }, + { + .compatible = "mediatek,mt8192-mmsys", + .data = &mt8192_mmsys_driver_data, + }, { } }; -- 1.8.1.1.dirty
[PATCH v8, 1/2] soc: mediatek: mmsys: add comp OVL_2L2/POSTMASK/RDMA4
This patch add some more ddp component OVL_2L2 is ovl which include 2 layers overlay POSTMASK control round corner for display frame RDMA4 read dma buffer Signed-off-by: Yongqiang Niu Reviewed-by: Chun-Kuang Hu Reviewed-by: Enric Balletbo i Serra Signed-off-by: Yongqiang Niu --- include/linux/soc/mediatek/mtk-mmsys.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/soc/mediatek/mtk-mmsys.h b/include/linux/soc/mediatek/mtk-mmsys.h index 2228bf6..4bba275 100644 --- a/include/linux/soc/mediatek/mtk-mmsys.h +++ b/include/linux/soc/mediatek/mtk-mmsys.h @@ -29,13 +29,16 @@ enum mtk_ddp_comp_id { DDP_COMPONENT_OVL0, DDP_COMPONENT_OVL_2L0, DDP_COMPONENT_OVL_2L1, + DDP_COMPONENT_OVL_2L2, DDP_COMPONENT_OVL1, + DDP_COMPONENT_POSTMASK0, DDP_COMPONENT_PWM0, DDP_COMPONENT_PWM1, DDP_COMPONENT_PWM2, DDP_COMPONENT_RDMA0, DDP_COMPONENT_RDMA1, DDP_COMPONENT_RDMA2, + DDP_COMPONENT_RDMA4, DDP_COMPONENT_UFOE, DDP_COMPONENT_WDMA0, DDP_COMPONENT_WDMA1, -- 1.8.1.1.dirty
Re: [PATCH 3/4] drm/tiny: add simple-dbi driver
Hi Icenowy, I love your patch! Yet something to improve: [auto build test ERROR on robh/for-next] [also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip linus/master v5.14-rc3 next-20210730] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Icenowy-Zheng/Add-simple-dbi-tinydrm-driver/20210802-144017 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 10.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 # https://github.com/0day-ci/linux/commit/257b1975790880588d1ec7cfec4ebde54d23d63a git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Icenowy-Zheng/Add-simple-dbi-tinydrm-driver/20210802-144017 git checkout 257b1975790880588d1ec7cfec4ebde54d23d63a # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash drivers/gpu/drm/tiny/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): In file included from include/linux/kallsyms.h:14, from include/linux/ftrace.h:12, from include/linux/kprobes.h:29, from include/linux/kgdb.h:19, from include/linux/fb.h:5, from include/linux/backlight.h:13, from drivers/gpu/drm/tiny/simple-dbi.c:9: >> include/linux/module.h:244:1: error: expected ',' or ';' before 'extern' 244 | extern typeof(name) __mod_##type##__##name##_device_table \ | ^~ drivers/gpu/drm/tiny/simple-dbi.c:148:1: note: in expansion of macro 'MODULE_DEVICE_TABLE' 148 | MODULE_DEVICE_TABLE(of, simple_dbi_of_match); | ^~~ vim +244 include/linux/module.h ^1da177e4c3f41 Linus Torvalds2005-04-16 240 cff26a51da5d20 Rusty Russell 2014-02-03 241 #ifdef MODULE cff26a51da5d20 Rusty Russell 2014-02-03 242 /* Creates an alias so file2alias.c can find device table. */ ^1da177e4c3f41 Linus Torvalds2005-04-16 243 #define MODULE_DEVICE_TABLE(type, name) \ 0bf8bf50eddc75 Matthias Kaehlcke 2017-07-24 @244 extern typeof(name) __mod_##type##__##name##_device_table \ cff26a51da5d20 Rusty Russell 2014-02-03 245__attribute__ ((unused, alias(__stringify(name cff26a51da5d20 Rusty Russell 2014-02-03 246 #else /* !MODULE */ cff26a51da5d20 Rusty Russell 2014-02-03 247 #define MODULE_DEVICE_TABLE(type, name) cff26a51da5d20 Rusty Russell 2014-02-03 248 #endif ^1da177e4c3f41 Linus Torvalds2005-04-16 249 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
On Mon, Aug 02, 2021 at 10:49:37AM +0200, Michel Dänzer wrote: > On 2021-08-02 9:59 a.m., Daniel Vetter wrote: > > On Fri, Jul 30, 2021 at 02:50:10PM +0200, Michel Dänzer wrote: > >> On 2021-07-30 12:25 p.m., Daniel Vetter wrote: > >>> On Thu, Jul 29, 2021 at 01:16:55AM -0700, Vivek Kasireddy wrote: > By separating the OUT_FENCE signalling from pageflip completion allows > a Guest compositor to start a new repaint cycle with a new buffer > instead of waiting for the old buffer to be free. > > This work is based on the idea/suggestion from Simon and Pekka. > > This capability can be a solution for this issue: > https://gitlab.freedesktop.org/wayland/weston/-/issues/514 > > Corresponding Weston MR: > https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/668 > >>> > >>> Uh I kinda wanted to discuss this a bit more before we jump into typing > >>> code, but well I guess not that much work yet. > >>> > >>> So maybe I'm not understanding the problem, but I think the fundamental > >>> underlying issue is that with KMS you can have at most 2 buffers > >>> in-flight, due to our queue depth limit of 1 pending flip. > >>> > >>> Unfortunately that means for virtual hw where it takes a few more > >>> steps/vblanks until the framebuffer actually shows up on screen and is > >>> scanned out, we suffer deeply. The usual fix for that is to drop the > >>> latency and increase throughput, and have more buffers in-flight. Which > >>> this patch tries to do. > >> > >> Per > >> https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_986797 , > >> IMO the underlying issue is actually that the guest compositor repaint > >> cycle is not aligned with the host compositor one. If they were aligned, > >> the problem would not occur even without allowing multiple page flips in > >> flight, and latency would be lower. > > > > Yeah my proposal here is under the premise that we do actually need to fix > > this with a deeper queue depth. > > > >>> Now I think where we go wrong here is that we're trying to hack this up by > >>> defining different semantics for the out-fence and for the drm-event. Imo > >>> that's wrong, they're both meant to show eactly the same thing: > >>> - when is the new frame actually visible to the user (as in, eyeballs in a > >>> human head, preferrably, not the time when we've handed the buffer off > >>> to the virtual hw) > >>> - when is the previous buffer no longer being used by the scanout hw > >>> > >>> We do cheat a bit right now in so far that we assume they're both the > >>> same, as in, panel-side latency is currently the compositor's problem to > >>> figure out. > >>> > >>> So for virtual hw I think the timestamp and even completion really need to > >>> happen only when the buffer has been pushed through the entire > >>> virtualization chain, i.e. ideally we get the timestamp from the kms > >>> driver from the host side. Currently that's not done, so this is most > >>> likely quite broken already (virtio relies on the no-vblank auto event > >>> sending, which definitely doesn't wait for anything, or I'm completely > >>> missing something). > >>> > >>> I think instead of hacking up some ill-defined 1.5 queue depth support, > >>> what we should do is support queue depth > 1 properly. So: > >>> > >>> - Change atomic to support queue depth > 1, this needs to be a per-driver > >>> thing due to a bunch of issues in driver code. Essentially drivers must > >>> never look at obj->state pointers, and only ever look up state through > >>> the passed-in drm_atomic_state * update container. > >>> > >>> - Aside: virtio should loose all it's empty hooks, there's no point in > >>> that. > >>> > >>> - We fix virtio to send out the completion event at the end of this entire > >>> pipeline, i.e. virtio code needs to take care of sending out the > >>> crtc_state->event correctly. > >>> > >>> - We probably also want some kind of (maybe per-crtc) recommended queue > >>> depth property so compositors know how many buffers to keep in flight. > >>> Not sure about that. > >> > >> I'd say there would definitely need to be some kind of signal for the > >> display server that it should queue multiple flips, since this is > >> normally not desirable for latency. In other words, this wouldn't really > >> be useful on bare metal (in contrast to the ability to replace a pending > >> flip with a newer one). > > > > Hm I was thinking that the compositor can tune this. If the round-trip > > latency (as measured by events) is too long to get full refresh rate, it > > can add more buffers to the queue. That's kinda why I think the returned > > event really must be accurate wrt actual display time (and old buffer > > release time), so that this computation in the compositor because a pretty > > simple > > > > num_buffers = (flip_time - submit_time) / frame_time > > > > With maybe some rounding up and averaging. You can also hit this when your > > 3
Re: [PATCH 3/4] drm/tiny: add simple-dbi driver
在 2021-08-02星期一的 14:35 +0800,Icenowy Zheng写道: > Add a driver for generic MIPI DBI panels initialized with MIPI DCS > commands. > > Currently a ST7789V-based panel is added to it. This panel has its > configuration pre-programmed into the controller, so no vendor- > specific > configuration is needed. > > Signed-off-by: Icenowy Zheng > --- > drivers/gpu/drm/tiny/Kconfig | 12 ++ > drivers/gpu/drm/tiny/Makefile | 1 + > drivers/gpu/drm/tiny/simple-dbi.c | 244 > ++ > 3 files changed, 257 insertions(+) > create mode 100644 drivers/gpu/drm/tiny/simple-dbi.c > > diff --git a/drivers/gpu/drm/tiny/Kconfig > b/drivers/gpu/drm/tiny/Kconfig > index d31be274a2bd..6cfc57b68a46 100644 > --- a/drivers/gpu/drm/tiny/Kconfig > +++ b/drivers/gpu/drm/tiny/Kconfig > @@ -144,6 +144,18 @@ config TINYDRM_REPAPER > > If M is selected the module will be called repaper. > > +config TINYDRM_SIMPLE_DBI > + tristate "DRM support for generic MIPI DBI panels" > + depends on DRM && SPI > + select DRM_KMS_HELPER > + select DRM_KMS_CMA_HELPER > + select DRM_MIPI_DBI > + help > + DRM driver for generic DBI panels that are MIPI DCS > compatible > + and needs no vendor-specific setup commands. > + > + If M is selected the module will be called simple-dbi. > + > config TINYDRM_ST7586 > tristate "DRM support for Sitronix ST7586 display panels" > depends on DRM && SPI > diff --git a/drivers/gpu/drm/tiny/Makefile > b/drivers/gpu/drm/tiny/Makefile > index e09942895c77..2553de651aa3 100644 > --- a/drivers/gpu/drm/tiny/Makefile > +++ b/drivers/gpu/drm/tiny/Makefile > @@ -13,3 +13,4 @@ obj-$(CONFIG_TINYDRM_MI0283QT)+= > mi0283qt.o > obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o > obj-$(CONFIG_TINYDRM_ST7586) += st7586.o > obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o > +obj-$(CONFIG_TINYDRM_SIMPLE_DBI) += simple-dbi.o > diff --git a/drivers/gpu/drm/tiny/simple-dbi.c > b/drivers/gpu/drm/tiny/simple-dbi.c > new file mode 100644 > index ..2b207e43d500 > --- /dev/null > +++ b/drivers/gpu/drm/tiny/simple-dbi.c > @@ -0,0 +1,244 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * DRM driver for display panels with configuration preset and needs > only > + * standard MIPI DCS commands to bring up. > + * > + * Copyright (C) 2021 Sipeed > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define MIPI_DCS_ADDRESS_MODE_BGR BIT(3) > +#define MIPI_DCS_ADDRESS_MODE_REVERSE BIT(5) > +#define MIPI_DCS_ADDRESS_MODE_RTL BIT(6) > +#define MIPI_DCS_ADDRESS_MODE_BTT BIT(7) > + > +struct simple_dbi_cfg { > + const struct drm_display_mode mode; > + unsigned int left_offset; > + unsigned int top_offset; > + bool inverted; > + bool write_only; > + bool bgr; > + bool right_to_left; > + bool bottom_to_top; > +}; > + > +struct simple_dbi_priv { > + struct mipi_dbi_dev dbidev; /* Must be first for > .release() */ > + const struct simple_dbi_cfg *cfg; > +}; > + > +static void simple_dbi_pipe_enable(struct drm_simple_display_pipe > *pipe, > + struct drm_crtc_state *crtc_state, > + struct drm_plane_state *plane_state) > +{ > + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe- > >crtc.dev); > + struct simple_dbi_priv *priv = container_of(dbidev, > + struct > simple_dbi_priv, > + dbidev); > + struct mipi_dbi *dbi = &dbidev->dbi; > + int ret, idx; > + u8 addr_mode = 0x00; > + > + if (!drm_dev_enter(pipe->crtc.dev, &idx)) > + return; > + > + DRM_DEBUG_KMS("\n"); > + > + ret = mipi_dbi_poweron_reset(dbidev); > + if (ret) > + goto out_exit; > + > + mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE); > + msleep(5); > + > + /* Currently tinydrm supports 16bit only now */ > + mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, > + MIPI_DCS_PIXEL_FMT_16BIT); > + > + if (priv->cfg->inverted) > + mipi_dbi_command(dbi, MIPI_DCS_ENTER_INVERT_MODE); > + else > + mipi_dbi_command(dbi, MIPI_DCS_EXIT_INVERT_MODE); > + > + if (priv->cfg->bgr) > + addr_mode |= MIPI_DCS_ADDRESS_MODE_BGR; > + > + if (priv->cfg->right_to_left) > + addr_mode |= MIPI_DCS_ADDRESS_MODE_RTL; > + > + if (priv->cfg->bottom_to_top) > + addr_mode |= MIPI_DCS_ADDRESS_MODE_BTT; > + > + switch (dbidev->rotation) { > + default: > + break; > + case 90: > +
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
On Fri, Jul 30, 2021 at 03:38:50PM +0200, Gerd Hoffmann wrote: > Hi, > > > - We fix virtio to send out the completion event at the end of this entire > > pipeline, i.e. virtio code needs to take care of sending out the > > crtc_state->event correctly. > > That sounds sensible to me. Fence the virtio commands, make sure (on > the host side) the command completes only when the work is actually done > not only submitted. Has recently been added to qemu for RESOURCE_FLUSH > (aka frontbuffer rendering) and doing the same for SET_SCANOUT (aka > pageflipping), then send vblank events to userspace on command > completion certainly makes sense. Hm how does this all work? At least drm/virtio uses drm_atomic_helper_dirtyfb, so both DIRTYFB ioctl and atomic flips all end up in the same driver path for everything. Or do you just combine the resource_flush with the flip as needed and let the host side figure it all out? From a quick read of virtgpu_plane.c that seems to be the case ... Also to make this work we don't just need the fence, we need the timestamp (in a clock domain the guest can correct for ofc) of the host side kms driver flip completion. If you just have the fence then the jitter from going through all the layers will most likely make it unusable. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] drm: Fix oops in damage self-tests by mocking damage property
Op 30-07-2021 om 11:52 schreef Daniel Vetter: > I've added a new check to make sure that drivers which insepct the > damage property have it set up correctly, but somehow missed that this > borke the damage selftest in the CI result noise. > > Fix it up by mocking enough of drm_device and drm_plane so we can call > drm_plane_enable_fb_damage_clips() to make the new check happy. > > Since there's a lot of duplicated mock code already copy-pasted into > each test I've also refactored this a bit to trim it down. > > Signed-off-by: Daniel Vetter > Fixes: c7fcbf251397 ("drm/plane: check that fb_damage is set up when used") > Cc: José Roberto de Souza (v1) > Cc: Ville Syrjälä > Cc: Gwan-gyeong Mun > Cc: José Roberto de Souza > Cc: Hans de Goede > Cc: Daniel Vetter > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > --- > .../drm/selftests/test-drm_damage_helper.c| 287 +- > 1 file changed, 71 insertions(+), 216 deletions(-) > > diff --git a/drivers/gpu/drm/selftests/test-drm_damage_helper.c > b/drivers/gpu/drm/selftests/test-drm_damage_helper.c > index 9d2bcdf8bc29..1b585c13e042 100644 > --- a/drivers/gpu/drm/selftests/test-drm_damage_helper.c > +++ b/drivers/gpu/drm/selftests/test-drm_damage_helper.c > @@ -6,9 +6,37 @@ > #define pr_fmt(fmt) "drm_damage_helper: " fmt > > #include > +#include > +#include > > #include "test-drm_modeset_common.h" > > +struct drm_driver mock_driver; > +struct drm_device mock_device; > +struct drm_object_properties mock_obj_props; > +struct drm_plane mock_plane; > +struct drm_property mock_prop; > + > +static void mock_setup(struct drm_plane_state *state) > +{ > + static bool setup_done = false; > + > + state->plane = &mock_plane; > + > + if (setup_done) > + return; > + > + /* just enough so that drm_plane_enable_fb_damage_clips() works */ > + mock_device.driver = &mock_driver; > + mock_device.mode_config.prop_fb_damage_clips = &mock_prop; > + mock_plane.dev = &mock_device; > + mock_plane.base.properties = &mock_obj_props; > + mock_prop.base.id = 1; /* 0 is an invalid id */ > + mock_prop.dev = &mock_device; > + > + drm_plane_enable_fb_damage_clips(&mock_plane); > +} > + > static void set_plane_src(struct drm_plane_state *state, int x1, int y1, int > x2, > int y2) > { > @@ -70,23 +98,29 @@ static bool check_damage_clip(struct drm_plane_state > *state, struct drm_rect *r, > return true; > } > > +const struct drm_framebuffer fb = { > + .width = 2048, > + .height = 2048 > +}; > + > +/* common mocked structs many tests need */ > +#define MOCK_VARIABLES() \ > + struct drm_plane_state old_state; \ > + struct drm_plane_state state = { \ > + .crtc = ZERO_SIZE_PTR, \ > + .fb = (struct drm_framebuffer *) &fb, \ > + .visible = true, \ > + }; \ > + mock_setup(&old_state); \ > + mock_setup(&state); > + > int igt_damage_iter_no_damage(void *ignored) > { > struct drm_atomic_helper_damage_iter iter; > - struct drm_plane_state old_state; > struct drm_rect clip; > uint32_t num_hits = 0; > > - struct drm_framebuffer fb = { > - .width = 2048, > - .height = 2048 > - }; > - > - struct drm_plane_state state = { > - .crtc = ZERO_SIZE_PTR, > - .fb = &fb, > - .visible = true, > - }; > + MOCK_VARIABLES(); > > /* Plane src same as fb size. */ > set_plane_src(&old_state, 0, 0, fb.width << 16, fb.height << 16); > @@ -104,20 +138,10 @@ int igt_damage_iter_no_damage(void *ignored) > int igt_damage_iter_no_damage_fractional_src(void *ignored) > { > struct drm_atomic_helper_damage_iter iter; > - struct drm_plane_state old_state; > struct drm_rect clip; > uint32_t num_hits = 0; > > - struct drm_framebuffer fb = { > - .width = 2048, > - .height = 2048 > - }; > - > - struct drm_plane_state state = { > - .crtc = ZERO_SIZE_PTR, > - .fb = &fb, > - .visible = true, > - }; > + MOCK_VARIABLES(); > > /* Plane src has fractional part. */ > set_plane_src(&old_state, 0x3fffe, 0x3fffe, > @@ -137,20 +161,10 @@ int igt_damage_iter_no_damage_fractional_src(void > *ignored) > int igt_damage_iter_no_damage_src_moved(void *ignored) > { > struct drm_atomic_helper_damage_iter iter; > - struct drm_plane_state old_state; > struct drm_rect clip; > uint32_t num_hits = 0; > > - struct drm_framebuffer fb = { > - .width = 2048, > - .height = 2048 > - }; > - > - struct drm_plane_state state = { > - .crtc = ZERO_SIZE_PTR, > - .fb = &fb, > - .visible = true, > - }; > + MOCK_VARIABLES(); > > /* Plane src moved since old plane state. */ > set_plane_src(&old_state, 0, 0, 1024 << 16,
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
On 2021-08-02 11:06 a.m., Daniel Vetter wrote: > On Mon, Aug 02, 2021 at 10:49:37AM +0200, Michel Dänzer wrote: >> On 2021-08-02 9:59 a.m., Daniel Vetter wrote: >>> On Fri, Jul 30, 2021 at 02:50:10PM +0200, Michel Dänzer wrote: On 2021-07-30 12:25 p.m., Daniel Vetter wrote: > On Thu, Jul 29, 2021 at 01:16:55AM -0700, Vivek Kasireddy wrote: >> By separating the OUT_FENCE signalling from pageflip completion allows >> a Guest compositor to start a new repaint cycle with a new buffer >> instead of waiting for the old buffer to be free. >> >> This work is based on the idea/suggestion from Simon and Pekka. >> >> This capability can be a solution for this issue: >> https://gitlab.freedesktop.org/wayland/weston/-/issues/514 >> >> Corresponding Weston MR: >> https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/668 > > Uh I kinda wanted to discuss this a bit more before we jump into typing > code, but well I guess not that much work yet. > > So maybe I'm not understanding the problem, but I think the fundamental > underlying issue is that with KMS you can have at most 2 buffers > in-flight, due to our queue depth limit of 1 pending flip. > > Unfortunately that means for virtual hw where it takes a few more > steps/vblanks until the framebuffer actually shows up on screen and is > scanned out, we suffer deeply. The usual fix for that is to drop the > latency and increase throughput, and have more buffers in-flight. Which > this patch tries to do. Per https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_986797 , IMO the underlying issue is actually that the guest compositor repaint cycle is not aligned with the host compositor one. If they were aligned, the problem would not occur even without allowing multiple page flips in flight, and latency would be lower. >>> >>> Yeah my proposal here is under the premise that we do actually need to fix >>> this with a deeper queue depth. >>> > Now I think where we go wrong here is that we're trying to hack this up by > defining different semantics for the out-fence and for the drm-event. Imo > that's wrong, they're both meant to show eactly the same thing: > - when is the new frame actually visible to the user (as in, eyeballs in a > human head, preferrably, not the time when we've handed the buffer off > to the virtual hw) > - when is the previous buffer no longer being used by the scanout hw > > We do cheat a bit right now in so far that we assume they're both the > same, as in, panel-side latency is currently the compositor's problem to > figure out. > > So for virtual hw I think the timestamp and even completion really need to > happen only when the buffer has been pushed through the entire > virtualization chain, i.e. ideally we get the timestamp from the kms > driver from the host side. Currently that's not done, so this is most > likely quite broken already (virtio relies on the no-vblank auto event > sending, which definitely doesn't wait for anything, or I'm completely > missing something). > > I think instead of hacking up some ill-defined 1.5 queue depth support, > what we should do is support queue depth > 1 properly. So: > > - Change atomic to support queue depth > 1, this needs to be a per-driver > thing due to a bunch of issues in driver code. Essentially drivers must > never look at obj->state pointers, and only ever look up state through > the passed-in drm_atomic_state * update container. > > - Aside: virtio should loose all it's empty hooks, there's no point in > that. > > - We fix virtio to send out the completion event at the end of this entire > pipeline, i.e. virtio code needs to take care of sending out the > crtc_state->event correctly. > > - We probably also want some kind of (maybe per-crtc) recommended queue > depth property so compositors know how many buffers to keep in flight. > Not sure about that. I'd say there would definitely need to be some kind of signal for the display server that it should queue multiple flips, since this is normally not desirable for latency. In other words, this wouldn't really be useful on bare metal (in contrast to the ability to replace a pending flip with a newer one). >>> >>> Hm I was thinking that the compositor can tune this. If the round-trip >>> latency (as measured by events) is too long to get full refresh rate, it >>> can add more buffers to the queue. That's kinda why I think the returned >>> event really must be accurate wrt actual display time (and old buffer >>> release time), so that this computation in the compositor because a pretty >>> simple >>> >>> num_buffers = (flip_time - submit_time) / frame_time >>> >>> With maybe some rounding up and ave
Re: [PATCH v2 4/4] ARM: dts: add SKOV imx6q and imx6dl based boards
[...] > > + reg_vcc_mmc_io: regulator-vcc-mmc-io { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_vcc_mmc_io>; > > + compatible = "regulator-gpio"; > > + vin-supply = <®_5v0>; > > + regulator-name = "mmc_io_supply"; > > + regulator-type = "voltage"; > > + regulator-min-microvolt = <180>; > > + regulator-max-microvolt = <330>; > > + gpios = <&gpio7 13 GPIO_ACTIVE_HIGH>; > > enable-active-high? right, thx! > > + states = <180 0x1>, <330 0x0>; > > Hmm, I do not see this 'states' in fixed-regulator.yaml. It is in gpio-regulator.yaml Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH] drm/radeon: Update pitch for page flip
When primary bo is updated, crtc's pitch may have not been updated, this will lead to show disorder content when user changes display mode, we update crtc's pitch in page flip to avoid this bug. This refers to amdgpu's pageflip. Cc: Alex Deucher Cc: "Christian König" Cc: "Pan, Xinhui" Cc: David Airlie Cc: Daniel Vetter Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Signed-off-by: Zhenneng Li --- drivers/gpu/drm/radeon/evergreen.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 36a888e1b179..eeb590d2dec2 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -28,6 +28,7 @@ #include #include +#include #include "atom.h" #include "avivod.h" @@ -1414,10 +1415,15 @@ void evergreen_page_flip(struct radeon_device *rdev, int crtc_id, u64 crtc_base, bool async) { struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id]; + struct drm_framebuffer *fb = radeon_crtc->base.primary->fb; - /* update the scanout addresses */ + /* flip at hsync for async, default is vsync */ WREG32(EVERGREEN_GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset, async ? EVERGREEN_GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0); + /* update pitch */ + WREG32(EVERGREEN_GRPH_PITCH + radeon_crtc->crtc_offset, + fb->pitches[0] / fb->format->cpp[0]); + /* update the scanout addresses */ WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + radeon_crtc->crtc_offset, upper_32_bits(crtc_base)); WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + radeon_crtc->crtc_offset, -- 2.25.1 Content-type: Text/plain No virus found Checked by Hillstone Network AntiVirus
Re: [PATCH v2 1/2] drm/i915: document caching related bits
Matthew Auld writes: > Try to document the object caching related bits, like cache_coherent and > cache_dirty. > > v2(Ville): > - As pointed out by Ville, fix the completely incorrect assumptions >about the "partial" coherency on shared LLC platforms. > > Suggested-by: Daniel Vetter > Signed-off-by: Matthew Auld > Cc: Ville Syrjälä > Cc: Mika Kuoppala > --- > .../gpu/drm/i915/gem/i915_gem_object_types.h | 173 +- > drivers/gpu/drm/i915/i915_drv.h | 9 - > 2 files changed, 169 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > index ef3de2ae9723..a809424bc8c1 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h > @@ -92,6 +92,76 @@ struct drm_i915_gem_object_ops { > const char *name; /* friendly name for debug, e.g. lockdep classes */ > }; > > +/** > + * enum i915_cache_level - The supported GTT caching values for system memory > + * pages. > + * > + * These translate to some special GTT PTE bits when binding pages into some > + * address space. It also determines whether an object, or rather its pages > are > + * coherent with the GPU, when also reading or writing through the CPU cache > + * with those pages. > + * > + * Userspace can also control this through struct drm_i915_gem_caching. > + */ > +enum i915_cache_level { > + /** > + * @I915_CACHE_NONE: > + * > + * Not coherent with the CPU cache. If the cache is dirty and we need > + * the underlying pages to be coherent with some later GPU access then > + * we need to manually flush the pages. > + * > + * Note that on shared LLC platforms reads and writes through the CPU > + * cache are still coherent even with this setting. See also > + * &drm_i915_gem_object.cache_coherent for more details. > + * > + * Note that on platforms with a shared LLC this should ideally only be > + * used for scanout surfaces, otherwise we end up over-flushing in some > + * places. > + */ > + I915_CACHE_NONE = 0, > + /** > + * @I915_CACHE_LLC: > + * > + * Coherent with the CPU cache. If the cache is dirty, then the GPU will > + * ensure that access remains coherent, when both reading and writing > + * through the CPU cache. > + * > + * Not used for scanout surfaces. > + * > + * Applies to both platforms with shared LLC(HAS_LLC), and snooping > + * based platforms(HAS_SNOOP). > + * > + * This should be the default for platforms which share the LLC with the > + * CPU. The only exception is scanout objects, where the display engine > + * is not coherent with the LLC. For such objects I915_CACHE_NONE or > + * I915_CACHE_WT should be used. > + */ > + I915_CACHE_LLC, > + /** > + * @I915_CACHE_L3_LLC: > + * > + * Explicitly enable the Gfx L3 cache, with snooped LLC. > + * > + * The Gfx L3 sits between the domain specific caches, e.g > + * sampler/render caches, and the larger LLC. LLC is coherent with the > + * GPU, but L3 is only visible to the GPU, so likely needs to be flushed > + * when the workload completes. > + * > + * Not used for scanout surfaces. > + * > + * Only exposed on some gen7 + GGTT. More recent hardware has dropped > + * this. > + */ This is stellar. Thanks! -Mika > + I915_CACHE_L3_LLC, > + /** > + * @I915_CACHE_WT: > + * > + * hsw:gt3e Write-through for scanout buffers. > + */ > + I915_CACHE_WT, > +}; > + > enum i915_map_type { > I915_MAP_WB = 0, > I915_MAP_WC, > @@ -228,14 +298,109 @@ struct drm_i915_gem_object { > unsigned int mem_flags; > #define I915_BO_FLAG_STRUCT_PAGE BIT(0) /* Object backed by struct pages */ > #define I915_BO_FLAG_IOMEM BIT(1) /* Object backed by IO memory */ > - /* > - * Is the object to be mapped as read-only to the GPU > - * Only honoured if hardware has relevant pte bit > + /** > + * @cache_level: The desired GTT caching level. > + * > + * See enum i915_cache_level for possible values, along with what > + * each does. >*/ > unsigned int cache_level:3; > - unsigned int cache_coherent:2; > + /** > + * @cache_coherent: > + * > + * Track whether the pages are coherent with the GPU if reading or > + * writing through the CPU caches. The largely depends on the > + * @cache_level setting. > + * > + * On platforms which don't have the shared LLC(HAS_SNOOP), like on Atom > + * platforms, coherency must be explicitly requested with some special > + * GTT caching bits(see enum i915_cache_level). When enabling coherency > + * it does come at a performance and power cost on such platforms. On > + * the flip side the ker
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Check if engine has heartbeat when closing a context
On 30/07/2021 19:13, John Harrison wrote: On 7/30/2021 02:49, Tvrtko Ursulin wrote: On 30/07/2021 01:13, John Harrison wrote: On 7/28/2021 17:34, Matthew Brost wrote: If an engine associated with a context does not have a heartbeat, ban it immediately. This is needed for GuC submission as a idle pulse doesn't kick the context off the hardware where it then can check for a heartbeat and ban the context. Pulse, that is a request with I915_PRIORITY_BARRIER, does not preempt a running normal priority context? Why does it matter then whether or not heartbeats are enabled - when heartbeat just ends up sending the same engine pulse (eventually, with raising priority)? The point is that the pulse is pointless. See the rest of my comments below, specifically "the context will get resubmitted to the hardware after the pulse completes". To re-iterate... Yes, it preempts the context. Yes, it does so whether heartbeats are enabled or not. But so what? Who cares? You have preempted a context. It is no longer running on the hardware. BUT IT IS STILL A VALID CONTEXT. It is valid yes, and it even may be the current ABI so another question is whether it is okay to change that. The backend scheduler will just resubmit it to the hardware as soon as the pulse completes. The only reason this works at all is because of the horrid hack in the execlist scheduler's back end implementation (in __execlists_schedule_in): if (unlikely(intel_context_is_closed(ce) && !intel_engine_has_heartbeat(engine))) intel_context_set_banned(ce); Right, is the above code then needed with this patch - when ban is immediately applied on the higher level? The actual back end scheduler is saying "Is this a zombie context? Is the heartbeat disabled? Then ban it". No other scheduler backend is going to have knowledge of zombie context status or of the heartbeat status. Nor are they going to call back into the higher levels of the i915 driver to trigger a ban operation. Certainly a hardware implemented scheduler is not going to be looking at private i915 driver information to decide whether to submit a context or whether to tell the OS to kill it off instead. For persistence to work with a hardware scheduler (or a non-Intel specific scheduler such as the DRM one), the handling of zombie contexts, banning, etc. *must* be done entirely in the front end. It cannot rely on any backend hacks. That means you can't rely on any fancy behaviour of pulses. If you want to ban a context then you must explicitly ban that context. If you want to ban it at some later point then you need to track it at the top level as a zombie and then explicitly ban that zombie at whatever later point. I am still trying to understand it all. If I go by the commit message: """ This is needed for GuC submission as a idle pulse doesn't kick the context off the hardware where it then can check for a heartbeat and ban the context. """ That did not explain things for me. Sentence does not appear to make sense. Now, it seems "kick off the hardware" is meant as revoke and not just preempt. Which is fine, perhaps just needs to be written more explicitly. But the part of checking for heartbeat after idle pulse does not compute for me. It is the heartbeat which emits idle pulses, not idle pulse emitting heartbeats. But anyway, I can buy the handling at the front end story completely. It makes sense. We just need to agree that a) it is okay to change the ABI and b) remove the backend check from execlists if it is not needed any longer. And if ABI change is okay then commit message needs to talk about it loudly and clearly. Or perhaps there is no ABI change? I am not really clear how does setting banned status propagate to the GuC backend. I mean at which point does i915 ends up passing that info to the firmware? Regards, Tvrtko It's worse than this. If the engine in question is an individual physical engine then sending a pulse (with sufficiently high priority) will pre-empt the engine and kick the context off. However, the GuC Why it is different for physical vs virtual, aren't both just schedulable contexts with different engine masks for what GuC is concerned? Oh, is it a matter of needing to send pulses to all engines which comprise a virtual one? It isn't different. It is totally broken for both. It is potentially more broken for virtual engines because of the question of which engine to pulse. But as stated above, the pulse is pointless anyway so the which engine question doesn't even matter. John. scheduler does not have hacks in it to check the state of the heartbeat or whether a context is actually a zombie or not. Thus, the context will get resubmitted to the hardware after the pulse completes and effectively nothing will have happened. I would assume that the DRM scheduler which we are meant to be switching to for execlist as well as G
Re: [PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c
On Mon, Aug 02, 2021 at 10:26:16AM +0200, Daniel Vetter wrote: > On Sat, Jul 31, 2021 at 04:24:56PM +0800, Desmond Cheong Zhi Xi wrote: > > Hi, > > > > Following a discussion on the patch ("drm: use the lookup lock in > > drm_is_current_master") [1], Peter Zijlstra proposed new lockdep_assert > > helpers to make it convenient to compose lockdep checks together. > > > > This series includes the patch that introduces the new lockdep helpers, > > then utilizes these helpers in drm_is_current_master_locked in the > > following patch. > > > > v1 -> v2: > > Patch 2: > > - Updated the kerneldoc on the lock design of drm_file.master to explain > > the use of lockdep_assert(). As suggested by Boqun Feng. > > > > Link: > > https://lore.kernel.org/lkml/20210722092929.244629-2-desmondcheon...@gmail.com/ > > [1] > > Can you pls also cc: this to intel-gfx so the local CI there can pick it > up and verify? Just to check we got it all. Acked-by: Peter Zijlstra (Intel) Feel free to take it through the drm tree.
Re: [PATCH v2 1/2] locking/lockdep: Provide lockdep_assert{, _once}() helpers
Op 31-07-2021 om 10:24 schreef Desmond Cheong Zhi Xi: > From: Peter Zijlstra > > Extract lockdep_assert{,_once}() helpers to more easily write composite > assertions like, for example: > > lockdep_assert(lockdep_is_held(&drm_device.master_mutex) || > lockdep_is_held(&drm_file.master_lookup_lock)); > > Signed-off-by: Peter Zijlstra (Intel) > Signed-off-by: Desmond Cheong Zhi Xi > Acked-by: Boqun Feng > Acked-by: Waiman Long > --- > include/linux/lockdep.h | 41 + > 1 file changed, 21 insertions(+), 20 deletions(-) > > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h > index 5cf387813754..9fe165beb0f9 100644 > --- a/include/linux/lockdep.h > +++ b/include/linux/lockdep.h > @@ -306,31 +306,29 @@ extern void lock_unpin_lock(struct lockdep_map *lock, > struct pin_cookie); > > #define lockdep_depth(tsk) (debug_locks ? (tsk)->lockdep_depth : 0) > > -#define lockdep_assert_held(l) do { > \ > - WARN_ON(debug_locks && \ > - lockdep_is_held(l) == LOCK_STATE_NOT_HELD); \ > - } while (0) > +#define lockdep_assert(cond) \ > + do { WARN_ON(debug_locks && !(cond)); } while (0) > > -#define lockdep_assert_not_held(l) do {\ > - WARN_ON(debug_locks && \ > - lockdep_is_held(l) == LOCK_STATE_HELD); \ > - } while (0) > +#define lockdep_assert_once(cond)\ > + do { WARN_ON_ONCE(debug_locks && !(cond)); } while (0) > > -#define lockdep_assert_held_write(l) do {\ > - WARN_ON(debug_locks && !lockdep_is_held_type(l, 0));\ > - } while (0) > +#define lockdep_assert_held(l) \ > + lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD) > > -#define lockdep_assert_held_read(l) do {\ > - WARN_ON(debug_locks && !lockdep_is_held_type(l, 1));\ > - } while (0) > +#define lockdep_assert_not_held(l) \ > + lockdep_assert(lockdep_is_held(l) != LOCK_STATE_HELD) > > -#define lockdep_assert_held_once(l) do {\ > - WARN_ON_ONCE(debug_locks && !lockdep_is_held(l)); \ > - } while (0) > +#define lockdep_assert_held_write(l) \ > + lockdep_assert(lockdep_is_held_type(l, 0)) > > -#define lockdep_assert_none_held_once() do { > \ > - WARN_ON_ONCE(debug_locks && current->lockdep_depth);\ > - } while (0) > +#define lockdep_assert_held_read(l) \ > + lockdep_assert(lockdep_is_held_type(l, 1)) > + > +#define lockdep_assert_held_once(l) \ > + lockdep_assert_once(lockdep_is_held(l) != LOCK_STATE_NOT_HELD) > + > +#define lockdep_assert_none_held_once() \ > + lockdep_assert_once(!current->lockdep_depth) > > #define lockdep_recursing(tsk) ((tsk)->lockdep_recursion) > > @@ -407,6 +405,9 @@ extern int lock_is_held(const void *); > extern int lockdep_is_held(const void *); > #define lockdep_is_held_type(l, r) (1) > > +#define lockdep_assert(c)do { } while (0) > +#define lockdep_assert_once(c) do { } while (0) > + > #define lockdep_assert_held(l) do { (void)(l); } while > (0) > #define lockdep_assert_not_held(l) do { (void)(l); } while (0) > #define lockdep_assert_held_write(l) do { (void)(l); } while (0) All other macros seem to do (void)(c); in this case? Otherwise looks good.
Re: [PATCH v7 00/12] Clean up "mediatek,larb"
On Fri, Jul 30, 2021 at 10:52:26AM +0800, Yong Wu wrote: > .../display/mediatek/mediatek,disp.txt| 9 > .../bindings/media/mediatek-jpeg-decoder.yaml | 9 > .../bindings/media/mediatek-jpeg-encoder.yaml | 9 > .../bindings/media/mediatek-mdp.txt | 8 > .../bindings/media/mediatek-vcodec.txt| 4 -- > arch/arm/boot/dts/mt2701.dtsi | 2 - > arch/arm/boot/dts/mt7623n.dtsi| 5 -- > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 --- > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 6 --- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 9 +++- > drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 +++- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 15 +++--- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 36 +-- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 - > drivers/gpu/drm/mediatek/mtk_drm_drv.c| 5 +- > drivers/iommu/mtk_iommu.c | 24 +- > drivers/iommu/mtk_iommu_v1.c | 31 - > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 45 +- > .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 2 - > drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 46 +-- > drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 2 - > drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 1 - > .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 37 ++- > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- > .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - > .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 44 ++ > drivers/memory/mtk-smi.c | 14 -- > include/soc/mediatek/smi.h| 20 > 28 files changed, 92 insertions(+), 321 deletions(-) So this is likely not going through the IOMMU tree, given Matthias has reviewed the IOMMU changes you can add my Acked-by: Joerg Roedel
Re: [PATCH 01/11] mm: Introduce a function to check for virtualization protection features
On Tue, Jul 27, 2021 at 05:26:04PM -0500, Tom Lendacky wrote: > In prep for other protected virtualization technologies, introduce a > generic helper function, prot_guest_has(), that can be used to check > for specific protection attributes, like memory encryption. This is > intended to eliminate having to add multiple technology-specific checks > to the code (e.g. if (sev_active() || tdx_active())). > > Co-developed-by: Andi Kleen > Signed-off-by: Andi Kleen > Co-developed-by: Kuppuswamy Sathyanarayanan > > Signed-off-by: Kuppuswamy Sathyanarayanan > > Signed-off-by: Tom Lendacky Reviewed-by: Joerg Roedel
Re: [PATCH 02/11] x86/sev: Add an x86 version of prot_guest_has()
On Tue, Jul 27, 2021 at 05:26:05PM -0500, Tom Lendacky wrote: > Introduce an x86 version of the prot_guest_has() function. This will be > used in the more generic x86 code to replace vendor specific calls like > sev_active(), etc. > > While the name suggests this is intended mainly for guests, it will > also be used for host memory encryption checks in place of sme_active(). > > The amd_prot_guest_has() function does not use EXPORT_SYMBOL_GPL for the > same reasons previously stated when changing sme_active(), sev_active and > sme_me_mask to EXPORT_SYBMOL: > commit 87df26175e67 ("x86/mm: Unbreak modules that rely on external > PAGE_KERNEL availability") > commit 9d5f38ba6c82 ("x86/mm: Unbreak modules that use the DMA API") > > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: Dave Hansen > Cc: Andy Lutomirski > Cc: Peter Zijlstra > Co-developed-by: Andi Kleen > Signed-off-by: Andi Kleen > Co-developed-by: Kuppuswamy Sathyanarayanan > > Signed-off-by: Kuppuswamy Sathyanarayanan > > Signed-off-by: Tom Lendacky Reviewed-by: Joerg Roedel
[PATCH v3 2/3] dt-bindings: arm: fsl: add SKOV imx6q and imx6dl based boards
Add SKOV imx6q/dl LT2, LT6 and mi1010ait-1cp1 boards. Signed-off-by: Oleksij Rempel --- Documentation/devicetree/bindings/arm/fsl.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml index e2097011c4b0..600dfe984ab3 100644 --- a/Documentation/devicetree/bindings/arm/fsl.yaml +++ b/Documentation/devicetree/bindings/arm/fsl.yaml @@ -221,6 +221,9 @@ properties: - prt,prti6q# Protonic PRTI6Q board - prt,prtwd2# Protonic WD2 board - rex,imx6q-rex-pro # Rex Pro i.MX6 Quad Board + - skov,imx6q-skov-revc-lt2 # SKOV IMX6 CPU QuadCore lt2 + - skov,imx6q-skov-revc-lt6 # SKOV IMX6 CPU QuadCore lt6 + - skov,imx6q-skov-reve-mi1010ait-1cp1 # SKOV IMX6 CPU QuadCore mi1010ait-1cp1 - solidrun,cubox-i/q# SolidRun Cubox-i Dual/Quad - solidrun,hummingboard/q - solidrun,hummingboard2/q @@ -378,6 +381,8 @@ properties: - prt,prtvt7# Protonic VT7 board - rex,imx6dl-rex-basic # Rex Basic i.MX6 Dual Lite Board - riot,imx6s-riotboard # RIoTboard i.MX6S + - skov,imx6dl-skov-revc-lt2 # SKOV IMX6 CPU SoloCore lt2 + - skov,imx6dl-skov-revc-lt6 # SKOV IMX6 CPU SoloCore lt6 - solidrun,cubox-i/dl# SolidRun Cubox-i Solo/DualLite - solidrun,hummingboard/dl - solidrun,hummingboard2/dl # SolidRun HummingBoard2 Solo/DualLite -- 2.30.2
[PATCH v3 1/3] dt-bindings: vendor-prefixes: Add an entry for SKOV A/S
Add "skov" entry for the SKOV A/S: https://www.skov.com/en/ Signed-off-by: Oleksij Rempel --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 62cb1d9341f5..738d4f44f0de 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -1068,6 +1068,8 @@ patternProperties: description: Silicon Integrated Systems Corp. "^sitronix,.*": description: Sitronix Technology Corporation + "^skov,.*": +description: SKOV A/S "^skyworks,.*": description: Skyworks Solutions, Inc. "^smartlabs,.*": -- 2.30.2
[PATCH v3 3/3] ARM: dts: add SKOV imx6q and imx6dl based boards
From: Sam Ravnborg Add SKOV imx6q/dl LT2, LT6 and mi1010ait-1cp1 boards. Signed-off-by: Sam Ravnborg Signed-off-by: Søren Andersen Signed-off-by: Juergen Borleis Signed-off-by: Ulrich Ölmann Signed-off-by: Michael Grzeschik Signed-off-by: Marco Felsch Signed-off-by: Lucas Stach Signed-off-by: Oleksij Rempel --- arch/arm/boot/dts/Makefile| 5 + arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts| 13 + arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts| 106 arch/arm/boot/dts/imx6q-skov-revc-lt2.dts | 36 ++ arch/arm/boot/dts/imx6q-skov-revc-lt6.dts | 128 + .../dts/imx6q-skov-reve-mi1010ait-1cp1.dts| 127 + arch/arm/boot/dts/imx6qdl-skov-cpu-revc.dtsi | 54 ++ arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi | 476 ++ 8 files changed, 945 insertions(+) create mode 100644 arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts create mode 100644 arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts create mode 100644 arch/arm/boot/dts/imx6q-skov-revc-lt2.dts create mode 100644 arch/arm/boot/dts/imx6q-skov-revc-lt6.dts create mode 100644 arch/arm/boot/dts/imx6q-skov-reve-mi1010ait-1cp1.dts create mode 100644 arch/arm/boot/dts/imx6qdl-skov-cpu-revc.dtsi create mode 100644 arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 828fefc9c436..ecff0743d35f 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -475,6 +475,8 @@ dtb-$(CONFIG_SOC_IMX6Q) += \ imx6dl-sabrelite.dtb \ imx6dl-sabresd.dtb \ imx6dl-savageboard.dtb \ + imx6dl-skov-revc-lt2.dtb \ + imx6dl-skov-revc-lt6.dtb \ imx6dl-solidsense.dtb \ imx6dl-ts4900.dtb \ imx6dl-ts7970.dtb \ @@ -576,6 +578,9 @@ dtb-$(CONFIG_SOC_IMX6Q) += \ imx6q-sabresd.dtb \ imx6q-savageboard.dtb \ imx6q-sbc6x.dtb \ + imx6q-skov-revc-lt2.dtb \ + imx6q-skov-revc-lt6.dtb \ + imx6q-skov-reve-mi1010ait-1cp1.dtb \ imx6q-solidsense.dtb \ imx6q-tbs2910.dtb \ imx6q-ts4900.dtb \ diff --git a/arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts b/arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts new file mode 100644 index ..667b8faa1807 --- /dev/null +++ b/arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +// +// Copyright (C) 2020 Pengutronix, Ulrich Oelmann + +/dts-v1/; +#include "imx6dl.dtsi" +#include "imx6qdl-skov-cpu.dtsi" +#include "imx6qdl-skov-cpu-revc.dtsi" + +/ { + model = "SKOV IMX6 CPU SoloCore"; + compatible = "skov,imx6dl-skov-revc-lt2", "fsl,imx6dl"; +}; diff --git a/arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts b/arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts new file mode 100644 index ..5dcc433fe2af --- /dev/null +++ b/arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +// +// Copyright (C) 2020 Pengutronix, Ulrich Oelmann + +/dts-v1/; +#include "imx6dl.dtsi" +#include "imx6qdl-skov-cpu.dtsi" +#include "imx6qdl-skov-cpu-revc.dtsi" + +/ { + model = "SKOV IMX6 CPU SoloCore"; + compatible = "skov,imx6dl-skov-revc-lt6", "fsl,imx6dl"; + + backlight: backlight { + compatible = "pwm-backlight"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_backlight>; + enable-gpios = <&gpio6 23 GPIO_ACTIVE_LOW>; + pwms = <&pwm2 0 2 0>; + brightness-levels = <0 255>; + num-interpolated-steps = <17>; + default-brightness-level = <8>; + power-supply = <®_24v0>; + }; + + display { + compatible = "fsl,imx-parallel-display"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_ipu1>; + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + display0_in: endpoint { + remote-endpoint = <&ipu1_di0_disp0>; + }; + }; + + port@1 { + reg = <1>; + + display0_out: endpoint { + remote-endpoint = <&panel_in>; + }; + }; + }; + + panel { + compatible = "logictechno,lttd800480070-l6wh-rt"; + backlight = <&backlight>; + power-supply = <®_3v3>; + + port { + panel_in: endpoint { + remote-endpoint = <&display0_out>; + }; + }; + }; +}; + +&ipu1_di0_disp0 { + remote-endpoint = <&display0_in>; +}; + +&iomuxc { + pinctrl_backlight: backlightgrp { + fsl,pins = < + MX6QDL_PAD_RGMII_TD3__GPIO6_IO230x58 + >; + }
[PATCH v3 0/3] Mainline imx6 based SKOV boards
changes v3: - drop panel bindings patches, it is already in drm-misc-next - remove some new lines - reorder compatibles at the start of the nodes - use lowercase for hex value - add enable-active-high to the regulator-vcc-mmc-io and fix MMC voltage configuration. changes v2: - remove unnecessary newlines. - change linux,wakeup to wakeup-source - change switch@3 unit-address to @0 - sort aliases alphabetically Mainline imx6 based DTs for SKOV A/S boards Oleksij Rempel (2): dt-bindings: vendor-prefixes: Add an entry for SKOV A/S dt-bindings: arm: fsl: add SKOV imx6q and imx6dl based boards Sam Ravnborg (1): ARM: dts: add SKOV imx6q and imx6dl based boards .../devicetree/bindings/arm/fsl.yaml | 5 + .../devicetree/bindings/vendor-prefixes.yaml | 2 + arch/arm/boot/dts/Makefile| 5 + arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts| 13 + arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts| 106 arch/arm/boot/dts/imx6q-skov-revc-lt2.dts | 36 ++ arch/arm/boot/dts/imx6q-skov-revc-lt6.dts | 128 + .../dts/imx6q-skov-reve-mi1010ait-1cp1.dts| 127 + arch/arm/boot/dts/imx6qdl-skov-cpu-revc.dtsi | 54 ++ arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi | 476 ++ 10 files changed, 952 insertions(+) create mode 100644 arch/arm/boot/dts/imx6dl-skov-revc-lt2.dts create mode 100644 arch/arm/boot/dts/imx6dl-skov-revc-lt6.dts create mode 100644 arch/arm/boot/dts/imx6q-skov-revc-lt2.dts create mode 100644 arch/arm/boot/dts/imx6q-skov-revc-lt6.dts create mode 100644 arch/arm/boot/dts/imx6q-skov-reve-mi1010ait-1cp1.dts create mode 100644 arch/arm/boot/dts/imx6qdl-skov-cpu-revc.dtsi create mode 100644 arch/arm/boot/dts/imx6qdl-skov-cpu.dtsi -- 2.30.2
Re: [PATCH 04/11] x86/sme: Replace occurrences of sme_active() with prot_guest_has()
On Tue, Jul 27, 2021 at 05:26:07PM -0500, Tom Lendacky wrote: > Replace occurrences of sme_active() with the more generic prot_guest_has() > using PATTR_HOST_MEM_ENCRYPT, except for in arch/x86/mm/mem_encrypt*.c > where PATTR_SME will be used. If future support is added for other memory > encryption technologies, the use of PATTR_HOST_MEM_ENCRYPT can be > updated, as required, to use PATTR_SME. > > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: Dave Hansen > Cc: Andy Lutomirski > Cc: Peter Zijlstra > Cc: Joerg Roedel > Cc: Will Deacon > Signed-off-by: Tom Lendacky Reviewed-by: Joerg Roedel
Re: [PATCH 05/11] x86/sev: Replace occurrences of sev_active() with prot_guest_has()
On Tue, Jul 27, 2021 at 05:26:08PM -0500, Tom Lendacky wrote: > Replace occurrences of sev_active() with the more generic prot_guest_has() > using PATTR_GUEST_MEM_ENCRYPT, except for in arch/x86/mm/mem_encrypt*.c > where PATTR_SEV will be used. If future support is added for other memory > encryption technologies, the use of PATTR_GUEST_MEM_ENCRYPT can be > updated, as required, to use PATTR_SEV. > > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: Dave Hansen > Cc: Andy Lutomirski > Cc: Peter Zijlstra > Cc: Ard Biesheuvel > Signed-off-by: Tom Lendacky Reviewed-by: Joerg Roedel
Re: [PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c
On 2/8/21 4:26 pm, Daniel Vetter wrote: On Sat, Jul 31, 2021 at 04:24:56PM +0800, Desmond Cheong Zhi Xi wrote: Hi, Following a discussion on the patch ("drm: use the lookup lock in drm_is_current_master") [1], Peter Zijlstra proposed new lockdep_assert helpers to make it convenient to compose lockdep checks together. This series includes the patch that introduces the new lockdep helpers, then utilizes these helpers in drm_is_current_master_locked in the following patch. v1 -> v2: Patch 2: - Updated the kerneldoc on the lock design of drm_file.master to explain the use of lockdep_assert(). As suggested by Boqun Feng. Link: https://lore.kernel.org/lkml/20210722092929.244629-2-desmondcheon...@gmail.com/ [1] Can you pls also cc: this to intel-gfx so the local CI there can pick it up and verify? Just to check we got it all. -Daniel Oops my bad, I missed out the CI for this series. Will resend with the proper cc. Best wishes, Desmond Best wishes, Desmond Desmond Cheong Zhi Xi (1): drm: add lockdep assert to drm_is_current_master_locked Peter Zijlstra (1): locking/lockdep: Provide lockdep_assert{,_once}() helpers drivers/gpu/drm/drm_auth.c | 6 +++--- include/drm/drm_file.h | 4 include/linux/lockdep.h| 41 +++--- 3 files changed, 28 insertions(+), 23 deletions(-) -- 2.25.1
Re: [PATCH 06/11] x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()
On Tue, Jul 27, 2021 at 05:26:09PM -0500, Tom Lendacky wrote: > @@ -48,7 +47,7 @@ static void sme_sev_setup_real_mode(struct > trampoline_header *th) > if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT)) > th->flags |= TH_FLAGS_SME_ACTIVE; > > - if (sev_es_active()) { > + if (prot_guest_has(PATTR_GUEST_PROT_STATE)) { > /* >* Skip the call to verify_cpu() in secondary_startup_64 as it >* will cause #VC exceptions when the AP can't handle them yet. Not sure how TDX will handle AP booting, are you sure it needs this special setup as well? Otherwise a check for SEV-ES would be better instead of the generic PATTR_GUEST_PROT_STATE. Regards, Joerg
Re: [PATCH 09/11] x86/sev: Remove the now unused mem_encrypt_active() function
On Tue, Jul 27, 2021 at 05:26:12PM -0500, Tom Lendacky wrote: > The mem_encrypt_active() function has been replaced by prot_guest_has(), > so remove the implementation. > > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Signed-off-by: Tom Lendacky Reviewed-by: Joerg Roedel
Re: [PATCH 08/11] mm: Remove the now unused mem_encrypt_active() function
On Tue, Jul 27, 2021 at 05:26:11PM -0500, Tom Lendacky wrote: > The mem_encrypt_active() function has been replaced by prot_guest_has(), > so remove the implementation. > > Signed-off-by: Tom Lendacky Reviewed-by: Joerg Roedel
Re: [PATCH 0/3] iommu/drm/msm: Allow non-coherent masters to use system cache
On Thu, Jul 29, 2021 at 10:08:22AM +0530, Sai Prakash Ranjan wrote: > On 2021-07-28 19:30, Georgi Djakov wrote: > > On Mon, Jan 11, 2021 at 07:45:02PM +0530, Sai Prakash Ranjan wrote: > > > commit ecd7274fb4cd ("iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag") > > > removed unused IOMMU_SYS_CACHE_ONLY prot flag and along with it went > > > the memory type setting required for the non-coherent masters to use > > > system cache. Now that system cache support for GPU is added, we will > > > need to set the right PTE attribute for GPU buffers to be sys cached. > > > Without this, the system cache lines are not allocated for GPU. > > > > > > So the patches in this series introduces a new prot flag IOMMU_LLC, > > > renames IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to IO_PGTABLE_QUIRK_PTW_LLC > > > and makes GPU the user of this protection flag. > > > > Thank you for the patchset! Are you planning to refresh it, as it does > > not apply anymore? > > > > I was waiting on Will's reply [1]. If there are no changes needed, then > I can repost the patch. I still think you need to handle the mismatched alias, no? You're adding a new memory type to the SMMU which doesn't exist on the CPU side. That can't be right. Will
[RESEND PATCH v2 0/2] locking/lockdep, drm: apply new lockdep assert in drm_auth.c
Hi all, My bad for the resend. Adding cc: intel-gfx, and the maintainers and mailing lists for include/drm/drm_file.h. Following a discussion on the patch ("drm: use the lookup lock in drm_is_current_master") [1], Peter Zijlstra proposed new lockdep_assert helpers to make it convenient to compose lockdep checks together. This series includes the patch that introduces the new lockdep helpers, then utilizes these helpers in drm_is_current_master_locked in the following patch. v1 -> v2: Patch 2: - Updated the kerneldoc on the lock design of drm_file.master to explain the use of lockdep_assert(). As suggested by Boqun Feng. Link: https://lore.kernel.org/lkml/20210722092929.244629-2-desmondcheon...@gmail.com/ [1] Best wishes, Desmond Desmond Cheong Zhi Xi (1): drm: add lockdep assert to drm_is_current_master_locked Peter Zijlstra (1): locking/lockdep: Provide lockdep_assert{,_once}() helpers drivers/gpu/drm/drm_auth.c | 6 +++--- include/drm/drm_file.h | 4 include/linux/lockdep.h| 41 +++--- 3 files changed, 28 insertions(+), 23 deletions(-) -- 2.25.1
[RESEND PATCH v2 1/2] locking/lockdep: Provide lockdep_assert{, _once}() helpers
From: Peter Zijlstra Extract lockdep_assert{,_once}() helpers to more easily write composite assertions like, for example: lockdep_assert(lockdep_is_held(&drm_device.master_mutex) || lockdep_is_held(&drm_file.master_lookup_lock)); Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Desmond Cheong Zhi Xi Acked-by: Boqun Feng Acked-by: Waiman Long Acked-by: Peter Zijlstra (Intel) --- include/linux/lockdep.h | 41 + 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index 5cf387813754..9fe165beb0f9 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -306,31 +306,29 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie); #define lockdep_depth(tsk) (debug_locks ? (tsk)->lockdep_depth : 0) -#define lockdep_assert_held(l) do {\ - WARN_ON(debug_locks && \ - lockdep_is_held(l) == LOCK_STATE_NOT_HELD); \ - } while (0) +#define lockdep_assert(cond) \ + do { WARN_ON(debug_locks && !(cond)); } while (0) -#define lockdep_assert_not_held(l) do {\ - WARN_ON(debug_locks && \ - lockdep_is_held(l) == LOCK_STATE_HELD); \ - } while (0) +#define lockdep_assert_once(cond) \ + do { WARN_ON_ONCE(debug_locks && !(cond)); } while (0) -#define lockdep_assert_held_write(l) do {\ - WARN_ON(debug_locks && !lockdep_is_held_type(l, 0));\ - } while (0) +#define lockdep_assert_held(l) \ + lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD) -#define lockdep_assert_held_read(l)do {\ - WARN_ON(debug_locks && !lockdep_is_held_type(l, 1));\ - } while (0) +#define lockdep_assert_not_held(l) \ + lockdep_assert(lockdep_is_held(l) != LOCK_STATE_HELD) -#define lockdep_assert_held_once(l)do {\ - WARN_ON_ONCE(debug_locks && !lockdep_is_held(l)); \ - } while (0) +#define lockdep_assert_held_write(l) \ + lockdep_assert(lockdep_is_held_type(l, 0)) -#define lockdep_assert_none_held_once()do { \ - WARN_ON_ONCE(debug_locks && current->lockdep_depth);\ - } while (0) +#define lockdep_assert_held_read(l)\ + lockdep_assert(lockdep_is_held_type(l, 1)) + +#define lockdep_assert_held_once(l)\ + lockdep_assert_once(lockdep_is_held(l) != LOCK_STATE_NOT_HELD) + +#define lockdep_assert_none_held_once()\ + lockdep_assert_once(!current->lockdep_depth) #define lockdep_recursing(tsk) ((tsk)->lockdep_recursion) @@ -407,6 +405,9 @@ extern int lock_is_held(const void *); extern int lockdep_is_held(const void *); #define lockdep_is_held_type(l, r) (1) +#define lockdep_assert(c) do { } while (0) +#define lockdep_assert_once(c) do { } while (0) + #define lockdep_assert_held(l) do { (void)(l); } while (0) #define lockdep_assert_not_held(l) do { (void)(l); } while (0) #define lockdep_assert_held_write(l) do { (void)(l); } while (0) -- 2.25.1
[RESEND PATCH v2 2/2] drm: add lockdep assert to drm_is_current_master_locked
In drm_is_current_master_locked, accessing drm_file.master should be protected by either drm_file.master_lookup_lock or drm_device.master_mutex. This was previously awkward to assert with lockdep. Following patch ("locking/lockdep: Provide lockdep_assert{,_once}() helpers"), this assertion is now convenient. So we add in the assertion and explain this lock design in the kerneldoc. Signed-off-by: Desmond Cheong Zhi Xi Acked-by: Boqun Feng Acked-by: Waiman Long Acked-by: Peter Zijlstra (Intel) --- drivers/gpu/drm/drm_auth.c | 6 +++--- include/drm/drm_file.h | 4 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 9c24b8cc8e36..6f4d7ff23c80 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -63,9 +63,9 @@ static bool drm_is_current_master_locked(struct drm_file *fpriv) { - /* Either drm_device.master_mutex or drm_file.master_lookup_lock -* should be held here. -*/ + lockdep_assert_once(lockdep_is_held(&fpriv->master_lookup_lock) || + lockdep_is_held(&fpriv->minor->dev->master_mutex)); + return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master; } diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 726cfe0ff5f5..a3acb7ac3550 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -233,6 +233,10 @@ struct drm_file { * this only matches &drm_device.master if the master is the currently * active one. * +* To update @master, both &drm_device.master_mutex and +* @master_lookup_lock need to be held, therefore holding either of +* them is safe and enough for the read side. +* * When dereferencing this pointer, either hold struct * &drm_device.master_mutex for the duration of the pointer's use, or * use drm_file_get_master() if struct &drm_device.master_mutex is not -- 2.25.1
Re: [PATCH] drm/vbox: Convert to Linux IRQ interfaces
Hi, On 7/6/21 9:50 AM, Thomas Zimmermann wrote: > Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's > IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers > don't benefit from using it. > > Signed-off-by: Thomas Zimmermann Thanks, patch looks good to me: Reviewed-by: Hans de Goede And to make sure things don't regress I've also given this a test spin: Tested-by: Hans de Goede Note I assume that you will push this out do drmi-misc yourself (if you've not done so already given that this patch is somewhat old). Regards, Hans > --- > drivers/gpu/drm/vboxvideo/vbox_drv.c | 1 - > drivers/gpu/drm/vboxvideo/vbox_drv.h | 1 - > drivers/gpu/drm/vboxvideo/vbox_irq.c | 16 +++- > 3 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c > b/drivers/gpu/drm/vboxvideo/vbox_drv.c > index 879a2445cc44..2b81cb259d23 100644 > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c > @@ -184,7 +184,6 @@ static const struct drm_driver driver = { > .lastclose = drm_fb_helper_lastclose, > > .fops = &vbox_fops, > - .irq_handler = vbox_irq_handler, > .name = DRIVER_NAME, > .desc = DRIVER_DESC, > .date = DRIVER_DATE, > diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.h > b/drivers/gpu/drm/vboxvideo/vbox_drv.h > index ac7c2effc46f..4903b91d7fe4 100644 > --- a/drivers/gpu/drm/vboxvideo/vbox_drv.h > +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.h > @@ -145,7 +145,6 @@ void vbox_mm_fini(struct vbox_private *vbox); > int vbox_irq_init(struct vbox_private *vbox); > void vbox_irq_fini(struct vbox_private *vbox); > void vbox_report_hotplug(struct vbox_private *vbox); > -irqreturn_t vbox_irq_handler(int irq, void *arg); > > /* vbox_hgsmi.c */ > void *hgsmi_buffer_alloc(struct gen_pool *guest_pool, size_t size, > diff --git a/drivers/gpu/drm/vboxvideo/vbox_irq.c > b/drivers/gpu/drm/vboxvideo/vbox_irq.c > index b3ded68603ba..903a6c48ee8b 100644 > --- a/drivers/gpu/drm/vboxvideo/vbox_irq.c > +++ b/drivers/gpu/drm/vboxvideo/vbox_irq.c > @@ -10,7 +10,8 @@ > */ > > #include > -#include > + > +#include > #include > > #include "vbox_drv.h" > @@ -31,7 +32,7 @@ void vbox_report_hotplug(struct vbox_private *vbox) > schedule_work(&vbox->hotplug_work); > } > > -irqreturn_t vbox_irq_handler(int irq, void *arg) > +static irqreturn_t vbox_irq_handler(int irq, void *arg) > { > struct drm_device *dev = (struct drm_device *)arg; > struct vbox_private *vbox = to_vbox_dev(dev); > @@ -170,16 +171,21 @@ static void vbox_hotplug_worker(struct work_struct > *work) > > int vbox_irq_init(struct vbox_private *vbox) > { > - struct pci_dev *pdev = to_pci_dev(vbox->ddev.dev); > + struct drm_device *dev = &vbox->ddev; > + struct pci_dev *pdev = to_pci_dev(dev->dev); > > INIT_WORK(&vbox->hotplug_work, vbox_hotplug_worker); > vbox_update_mode_hints(vbox); > > - return drm_irq_install(&vbox->ddev, pdev->irq); > + /* PCI devices require shared interrupts. */ > + return request_irq(pdev->irq, vbox_irq_handler, IRQF_SHARED, > dev->driver->name, dev); > } > > void vbox_irq_fini(struct vbox_private *vbox) > { > - drm_irq_uninstall(&vbox->ddev); > + struct drm_device *dev = &vbox->ddev; > + struct pci_dev *pdev = to_pci_dev(dev->dev); > + > + free_irq(pdev->irq, dev); > flush_work(&vbox->hotplug_work); > } >
[PATCH v3] drm: Fix typo in comments
fix typo for drm: *achived ==> achieved *userpace ==> userspace *commited ==> committed *depencies ==> dependencies *preceeding ==> preceding *similiar ==> similar *accidently ==> accidentally *transfered ==> transferred *openeing ==> opening *Propage ==> Propagate *interpretes ==> interprets *evry ==> every *interruptable ==> interruptible *Unfortunatley ==> Unfortunately *Deterine ==> Determine *non-existant ==> non-existent *satifying ==> satisfying *initalizing ==> initializing *possibily ==> possibly *accidently ==> accidentally *firmare ==> firmware *shuld ==> should *apperture ==> aperture *falure ==> failure *inforamtion ==> information *implemnt ==> implement *paramter ==> parameter *candiates ==> candidates *identifer ==> identifier *wil ==> will *overlayed ==> overlaid *accomodate ==> accommodate *uniqe ==> unique *intializes ==> initializes *useable ==> usable *arguement ==> argument *destry ==> destroy *savely ==> safely *Unsinged ==> Unsigned *unititialized ==> uninitialized *draing ==> drawing *invers ==> inverse *Enpoint ==> Endpointc *mimick ==> mimic *fo ==> for *functons ==> functions v1->v2: *respin with the change "iff ==> implies that" v2->v3: *revert the change of "iff" which means "if and only if" *remove "Reviewed-by:" *add other typo *update changelog Signed-off-by: Cai Huoqing --- drivers/gpu/drm/drm_aperture.c| 2 +- drivers/gpu/drm/drm_atomic.c | 2 +- drivers/gpu/drm/drm_atomic_helper.c | 14 +++--- drivers/gpu/drm/drm_atomic_uapi.c | 6 +++--- drivers/gpu/drm/drm_auth.c| 2 +- drivers/gpu/drm/drm_bridge.c | 2 +- drivers/gpu/drm/drm_bufs.c| 2 +- drivers/gpu/drm/drm_cache.c | 2 +- drivers/gpu/drm/drm_color_mgmt.c | 2 +- drivers/gpu/drm/drm_damage_helper.c | 2 +- drivers/gpu/drm/drm_dp_dual_mode_helper.c | 2 +- drivers/gpu/drm/drm_dp_helper.c | 8 drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- drivers/gpu/drm/drm_drv.c | 4 ++-- drivers/gpu/drm/drm_dsc.c | 2 +- drivers/gpu/drm/drm_edid.c| 4 ++-- drivers/gpu/drm/drm_fb_helper.c | 4 ++-- drivers/gpu/drm/drm_file.c| 6 +++--- drivers/gpu/drm/drm_format_helper.c | 2 +- drivers/gpu/drm/drm_framebuffer.c | 2 +- drivers/gpu/drm/drm_gem.c | 4 ++-- drivers/gpu/drm/drm_gem_atomic_helper.c | 4 ++-- drivers/gpu/drm/drm_gem_shmem_helper.c| 2 +- drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- drivers/gpu/drm/drm_ioctl.c | 4 ++-- drivers/gpu/drm/drm_irq.c | 2 +- drivers/gpu/drm/drm_mm.c | 2 +- drivers/gpu/drm/drm_mode_object.c | 2 +- drivers/gpu/drm/drm_modes.c | 4 ++-- drivers/gpu/drm/drm_plane.c | 2 +- drivers/gpu/drm/drm_plane_helper.c| 2 +- drivers/gpu/drm/drm_prime.c | 2 +- drivers/gpu/drm/drm_probe_helper.c| 2 +- drivers/gpu/drm/drm_property.c| 2 +- drivers/gpu/drm/drm_syncobj.c | 2 +- drivers/gpu/drm/drm_vblank.c | 6 +++--- 36 files changed, 58 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c index 9ac39cf11694..74bd4a76b253 100644 --- a/drivers/gpu/drm/drm_aperture.c +++ b/drivers/gpu/drm/drm_aperture.c @@ -78,7 +78,7 @@ * * Drivers that are susceptible to being removed by other drivers, such as * generic EFI or VESA drivers, have to register themselves as owners of their - * given framebuffer memory. Ownership of the framebuffer memory is achived + * given framebuffer memory. Ownership of the framebuffer memory is achieved * by calling devm_aperture_acquire_from_firmware(). On success, the driver * is the owner of the framebuffer range. The function fails if the * framebuffer is already by another driver. See below for an example. diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index d820423fac32..b127e30082ba 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -723,7 +723,7 @@ static void drm_atomic_plane_print_state(struct drm_printer *p, * clocks, scaler units, bandwidth and fifo limits shared among a group of * planes or CRTCs, and so on) it makes sense to model these as independent * objects. Drivers then need to do similar state tracking and commit ordering for - * such private (since not exposed to userpace) objects as the atomic core and + * such private (since not exposed to userspace) objects as the atomic core and * helpers already provide for connectors, planes and CRTCs. * * To make this easier on drivers the atomic core provides some support to track diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index f7bf1ea62d58..4d92036422ce 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b
Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()
Le 28/07/2021 à 00:26, Tom Lendacky a écrit : Replace occurrences of mem_encrypt_active() with calls to prot_guest_has() with the PATTR_MEM_ENCRYPT attribute. What about https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210730114231.23445-1-w...@kernel.org/ ? Christophe Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Borislav Petkov Cc: Dave Hansen Cc: Andy Lutomirski Cc: Peter Zijlstra Cc: David Airlie Cc: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: VMware Graphics Cc: Joerg Roedel Cc: Will Deacon Cc: Dave Young Cc: Baoquan He Signed-off-by: Tom Lendacky --- arch/x86/kernel/head64.c| 4 ++-- arch/x86/mm/ioremap.c | 4 ++-- arch/x86/mm/mem_encrypt.c | 5 ++--- arch/x86/mm/pat/set_memory.c| 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +++- drivers/gpu/drm/drm_cache.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 6 +++--- drivers/iommu/amd/iommu.c | 3 ++- drivers/iommu/amd/iommu_v2.c| 3 ++- drivers/iommu/iommu.c | 3 ++- fs/proc/vmcore.c| 6 +++--- kernel/dma/swiotlb.c| 4 ++-- 13 files changed, 29 insertions(+), 24 deletions(-) diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c index de01903c3735..cafed6456d45 100644 --- a/arch/x86/kernel/head64.c +++ b/arch/x86/kernel/head64.c @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include @@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long physaddr, * there is no need to zero it after changing the memory encryption * attribute. */ - if (mem_encrypt_active()) { + if (prot_guest_has(PATTR_MEM_ENCRYPT)) { vaddr = (unsigned long)__start_bss_decrypted; vaddr_end = (unsigned long)__end_bss_decrypted; for (; vaddr < vaddr_end; vaddr += PMD_SIZE) { diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c index 0f2d5ace5986..5e1c1f5cbbe8 100644 --- a/arch/x86/mm/ioremap.c +++ b/arch/x86/mm/ioremap.c @@ -693,7 +693,7 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr, bool arch_memremap_can_ram_remap(resource_size_t phys_addr, unsigned long size, unsigned long flags) { - if (!mem_encrypt_active()) + if (!prot_guest_has(PATTR_MEM_ENCRYPT)) return true; if (flags & MEMREMAP_ENC) @@ -723,7 +723,7 @@ pgprot_t __init early_memremap_pgprot_adjust(resource_size_t phys_addr, { bool encrypted_prot; - if (!mem_encrypt_active()) + if (!prot_guest_has(PATTR_MEM_ENCRYPT)) return prot; encrypted_prot = true; diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index 451de8e84fce..0f1533dbe81c 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -364,8 +364,7 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size) /* * SME and SEV are very similar but they are not the same, so there are * times that the kernel will need to distinguish between SME and SEV. The - * sme_active() and sev_active() functions are used for this. When a - * distinction isn't needed, the mem_encrypt_active() function can be used. + * sme_active() and sev_active() functions are used for this. * * The trampoline code is a good example for this requirement. Before * paging is activated, SME will access all memory as decrypted, but SEV @@ -451,7 +450,7 @@ void __init mem_encrypt_free_decrypted_mem(void) * The unused memory range was mapped decrypted, change the encryption * attribute from decrypted to encrypted before freeing it. */ - if (mem_encrypt_active()) { + if (sme_me_mask) { r = set_memory_encrypted(vaddr, npages); if (r) { pr_warn("failed to free unused decrypted pages\n"); diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c index ad8a5c586a35..6925f2bb4be1 100644 --- a/arch/x86/mm/pat/set_memory.c +++ b/arch/x86/mm/pat/set_memory.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -1986,7 +1987,7 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc) int ret; /* Nothing to do if memory encryption is not active */ - if (!mem_encrypt_active()) + if (!prot_guest_has(PATTR_MEM_ENCRYPT)) return 0; /* Should not be working on unaligned addresses */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index abb928894eac..8407224717df 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -38,6 +38,7 @@ #
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
Hi, > > That sounds sensible to me. Fence the virtio commands, make sure (on > > the host side) the command completes only when the work is actually done > > not only submitted. Has recently been added to qemu for RESOURCE_FLUSH > > (aka frontbuffer rendering) and doing the same for SET_SCANOUT (aka > > pageflipping), then send vblank events to userspace on command > > completion certainly makes sense. > > Hm how does this all work? At least drm/virtio uses > drm_atomic_helper_dirtyfb, so both DIRTYFB ioctl and atomic flips all end > up in the same driver path for everything. Or do you just combine the > resource_flush with the flip as needed and let the host side figure it all > out? From a quick read of virtgpu_plane.c that seems to be the case ... virtio_gpu_primary_plane_update() will send RESOURCE_FLUSH only for DIRTYFB and both SET_SCANOUT + RESOURCE_FLUSH for page-flip, and I think for the page-flip case the host (aka qemu) doesn't get the "wait until old framebuffer is not in use any more" right yet. So we'll need a host-side fix for that and a guest-side fix to switch from a blocking wait on the fence to vblank events. > Also to make this work we don't just need the fence, we need the timestamp > (in a clock domain the guest can correct for ofc) of the host side kms > driver flip completion. If you just have the fence then the jitter from > going through all the layers will most likely make it unusable. Well, there are no timestamps in the virtio-gpu protocol ... Also I'm not sure they would be that helpful, any timing is *much* less predictable in a virtual machine, especially in case the host machine is loaded. take care, Gerd
[PATCH] fbdev/efifb: Release PCI device's runtime PM ref during FB destroy
Atm the EFI FB driver gets a runtime PM reference for the associated GFX PCI device during driver probing and releases it only when removing the driver. When fbcon switches to the FB provided by the PCI device's driver (for instance i915/drmfb), the EFI FB will get only unregistered without the EFI FB driver getting unloaded, keeping the runtime PM reference acquired during driver probing. This reference will prevent the PCI driver from runtime suspending the device. Fix this by releasing the RPM reference from the EFI FB's destroy hook, called when the FB gets unregistered. Fixes: a6c0fd3d5a8b ("efifb: Ensure graphics device for efifb stays at PCI D0") Cc: Kai-Heng Feng Signed-off-by: Imre Deak --- drivers/video/fbdev/efifb.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c index 8ea8f079cde26..25cdea32b9633 100644 --- a/drivers/video/fbdev/efifb.c +++ b/drivers/video/fbdev/efifb.c @@ -47,6 +47,8 @@ static bool use_bgrt = true; static bool request_mem_succeeded = false; static u64 mem_flags = EFI_MEMORY_WC | EFI_MEMORY_UC; +static struct pci_dev *efifb_pci_dev; /* dev with BAR covering the efifb */ + static struct fb_var_screeninfo efifb_defined = { .activate = FB_ACTIVATE_NOW, .height = -1, @@ -243,6 +245,9 @@ static inline void efifb_show_boot_graphics(struct fb_info *info) {} static void efifb_destroy(struct fb_info *info) { + if (efifb_pci_dev) + pm_runtime_put(&efifb_pci_dev->dev); + if (info->screen_base) { if (mem_flags & (EFI_MEMORY_UC | EFI_MEMORY_WC)) iounmap(info->screen_base); @@ -333,7 +338,6 @@ ATTRIBUTE_GROUPS(efifb); static bool pci_dev_disabled; /* FB base matches BAR of a disabled device */ -static struct pci_dev *efifb_pci_dev; /* dev with BAR covering the efifb */ static struct resource *bar_resource; static u64 bar_offset; @@ -603,8 +607,6 @@ static int efifb_remove(struct platform_device *pdev) unregister_framebuffer(info); sysfs_remove_groups(&pdev->dev.kobj, efifb_groups); framebuffer_release(info); - if (efifb_pci_dev) - pm_runtime_put(&efifb_pci_dev->dev); return 0; } -- 2.27.0
Re: [PATCH v2 1/6] dt-bindings: drm/panel-simple: Introduce generic eDP panels
On Fri, 30 Jul 2021 14:26:20 -0700, Douglas Anderson wrote: > eDP panels generally contain almost everything needed to control them > in their EDID. This comes from their DP heritage were a computer needs > to be able to properly control pretty much any DP display that's > plugged into it. > > The one big issue with eDP panels and the reason that we need a panel > driver for them is that the power sequencing can be different per > panel. > > While it is true that eDP panel sequencing can be arbitrarily complex, > in practice it turns out that many eDP panels are compatible with just > some slightly different delays. See the contents of the bindings file > introduced in this patch for some details. > > The fact that eDP panels are 99% probable and that the power > sequencing (especially power up) can be compatible between many panels > means that there's a constant desire to plug multiple different panels > into the same board. This could be for second sourcing purposes or to > support multiple SKUs (maybe a 11" and a 13", for instance). > > As discussed [1], it should be OK to support this by adding two > properties to the device tree to specify the delays needed for > powering up the panel the first time. We'll create a new "edp-panel" > bindings file and define the two delays that might need to be > specified. NOTE: in the vast majority of the cases (HPD is hooked up > and isn't glitchy or is debounced) even these delays aren't needed. > > [1] > https://lore.kernel.org/r/CAD=FV=vzyompwqzzwdhjgh5cjjww_ecm-wqveivz-bdgxjp...@mail.gmail.com > > Signed-off-by: Douglas Anderson > --- > > Changes in v2: > - No longer allow fallback to panel-simple. > - Add "-ms" suffix to delays. > > .../bindings/display/panel/panel-edp.yaml | 188 ++ > 1 file changed, 188 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-edp.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/panel-edp.example.dt.yaml: bridge@2d: 'aux-bus' does not match any of the regexes: 'pinctrl-[0-9]+' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.yaml \ndoc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1511822 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125
https://bugzilla.kernel.org/show_bug.cgi?id=205089 --- Comment #17 from Alex Deucher (alexdeuc...@gmail.com) --- Does up/downgrading the mesa driver help? -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 213935] AMDGPU Renoir crash/freeze while using vaapi with some video types in some apps - drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out!
https://bugzilla.kernel.org/show_bug.cgi?id=213935 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- Can you try a newer or older version of mesa? Most likely this is a bug in the user mode driver. The kernel is just the messenger. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 205089] amdgpu : drm:amdgpu_cs_ioctl : Failed to initialize parser -125
https://bugzilla.kernel.org/show_bug.cgi?id=205089 --- Comment #18 from jes...@jnsn.dev --- On 02/08/21 at 02:13pm, bugzilla-dae...@bugzilla.kernel.org wrote: >Does up/downgrading the mesa driver help? Upgrading to the latest git revision of mesa has fixed Dota 2 for me at least. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
On Mon, Aug 2, 2021 at 2:51 PM Gerd Hoffmann wrote: > > Hi, > > > > That sounds sensible to me. Fence the virtio commands, make sure (on > > > the host side) the command completes only when the work is actually done > > > not only submitted. Has recently been added to qemu for RESOURCE_FLUSH > > > (aka frontbuffer rendering) and doing the same for SET_SCANOUT (aka > > > pageflipping), then send vblank events to userspace on command > > > completion certainly makes sense. > > > > Hm how does this all work? At least drm/virtio uses > > drm_atomic_helper_dirtyfb, so both DIRTYFB ioctl and atomic flips all end > > up in the same driver path for everything. Or do you just combine the > > resource_flush with the flip as needed and let the host side figure it all > > out? From a quick read of virtgpu_plane.c that seems to be the case ... > > virtio_gpu_primary_plane_update() will send RESOURCE_FLUSH only for > DIRTYFB and both SET_SCANOUT + RESOURCE_FLUSH for page-flip, and I > think for the page-flip case the host (aka qemu) doesn't get the > "wait until old framebuffer is not in use any more" right yet. Hm reading the code I think you simply elide the set_scanout if it's still the same buffer. There's no difference betweeen dirtyfb and an atomic commit that just hands the damage rects to the driver. At least if you use the helpers. > So we'll need a host-side fix for that and a guest-side fix to switch > from a blocking wait on the fence to vblank events. > > > Also to make this work we don't just need the fence, we need the timestamp > > (in a clock domain the guest can correct for ofc) of the host side kms > > driver flip completion. If you just have the fence then the jitter from > > going through all the layers will most likely make it unusable. > > Well, there are no timestamps in the virtio-gpu protocol ... > > Also I'm not sure they would be that helpful, any timing is *much* less > predictable in a virtual machine, especially in case the host machine is > loaded. Hm yeah if the output is currently not displaying, then the timestamp is very fake. But if you display you should be able to pass it all around in both direction. So target vblank (or whatever it's called) would go from guest to host to host-compositor (over wayland protocol) to host-side kms, and the timestamp could travel all the way back. But yeah making this all work correctly is going to be a pile of work. Also I have no idea how well compositors take it when a kms driver switches between high-precision timestamps and frame scheduling to the entirely virtual/vblank-less approach on the fly. -Daniel > take care, > Gerd > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH] drm/msm/dp: Remove unused variable
On Thu, Jul 15, 2021 at 3:09 AM Stephen Boyd wrote: > > Quoting Souptick Joarder (2021-07-08 19:48:34) > > Kernel test roobot throws below warning -> > > > > drivers/gpu/drm/msm/dp/dp_display.c:1017:21: > > warning: variable 'drm' set but not used [-Wunused-but-set-variable] > > > > Removed unused variable drm. > > Can we get this in queue for 5.15, if no further comment ? > > Reported-by: kernel test robot > > Signed-off-by: Souptick Joarder > > --- > > Reviewed-by: Stephen Boyd
Re: [PATCH] drm/radeon: Update pitch for page flip
On Mon, Aug 2, 2021 at 4:31 AM Daniel Vetter wrote: > > On Mon, Aug 02, 2021 at 10:12:47AM +0200, Christian König wrote: > > Am 02.08.21 um 09:43 schrieb Zhenneng Li: > > > When primary bo is updated, crtc's pitch may > > > have not been updated, this will lead to show > > > disorder content when user changes display mode, > > > we update crtc's pitch in page flip to avoid > > > this bug. > > > This refers to amdgpu's pageflip. > > > > Alex is the expert to ask about that code, but I'm not sure if that is > > really correct for the old hardware. > > > > As far as I know the crtc's pitch should not change during a page flip, but > > only during a full mode set. > > > > So could you elaborate a bit more how you trigger this? > > legacy page_flip ioctl only verifies that the fb->format stays the same. > It doesn't check anything else (afair never has), this is all up to > drivers to verify. > > Personally I'd say add a check to reject this, since testing this and > making sure it really works everywhere is probably a bit much on this old > hw. If just the pitch changed, that probably wouldn't be much of a problem, but if the pitch is changing, that probably implies other stuff has changed as well and we'll just be chasing changes. I agree it would be best to just reject anything other than updating the scanout address. Alex > -Daniel > > > > > Thanks, > > Christian. > > > > > > > > Cc: Alex Deucher > > > Cc: "Christian König" > > > Cc: "Pan, Xinhui" > > > Cc: David Airlie > > > Cc: Daniel Vetter > > > Cc: amd-...@lists.freedesktop.org > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: linux-ker...@vger.kernel.org > > > Signed-off-by: Zhenneng Li > > > --- > > > drivers/gpu/drm/radeon/evergreen.c | 8 +++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/radeon/evergreen.c > > > b/drivers/gpu/drm/radeon/evergreen.c > > > index 36a888e1b179..eeb590d2dec2 100644 > > > --- a/drivers/gpu/drm/radeon/evergreen.c > > > +++ b/drivers/gpu/drm/radeon/evergreen.c > > > @@ -28,6 +28,7 @@ > > > #include > > > #include > > > +#include > > > #include "atom.h" > > > #include "avivod.h" > > > @@ -1414,10 +1415,15 @@ void evergreen_page_flip(struct radeon_device > > > *rdev, int crtc_id, u64 crtc_base, > > > bool async) > > > { > > > struct radeon_crtc *radeon_crtc = rdev->mode_info.crtcs[crtc_id]; > > > + struct drm_framebuffer *fb = radeon_crtc->base.primary->fb; > > > - /* update the scanout addresses */ > > > + /* flip at hsync for async, default is vsync */ > > > WREG32(EVERGREEN_GRPH_FLIP_CONTROL + radeon_crtc->crtc_offset, > > >async ? EVERGREEN_GRPH_SURFACE_UPDATE_H_RETRACE_EN : 0); > > > + /* update pitch */ > > > + WREG32(EVERGREEN_GRPH_PITCH + radeon_crtc->crtc_offset, > > > + fb->pitches[0] / fb->format->cpp[0]); > > > + /* update the scanout addresses */ > > > WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS_HIGH + > > > radeon_crtc->crtc_offset, > > >upper_32_bits(crtc_base)); > > > WREG32(EVERGREEN_GRPH_PRIMARY_SURFACE_ADDRESS + > > > radeon_crtc->crtc_offset, > > > > > > No virus found > > > Checked by Hillstone Network AntiVirus > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
[PATCH] drm/msm/gpu: fix link failure with QCOM_SCM=m
From: Arnd Bergmann Another missed dependency when SCM is a loadable module and adreno is built-in: drivers/gpu/drm/msm/adreno/adreno_gpu.o: In function `adreno_zap_shader_load': adreno_gpu.c:(.text+0x1e8): undefined reference to `qcom_scm_is_available' drivers/gpu/drm/msm/adreno/a5xx_gpu.o: In function `a5xx_hw_init': a5xx_gpu.c:(.text+0x28a6): undefined reference to `qcom_scm_set_remote_state' Change it so the dependency on QCOM_SCM and QCOM_MDT_LOADER can be ignored if we are not building for ARCH_QCOM, but prevent the link error during compile testing when SCM is a loadable module and ARCH_QCOM is disabled. Fixes: a9e2559c931d ("drm/msm/gpu: Move zap shader loading to adreno") Fixes: 5ea4dba68305 ("drm/msm/a6xx: add CONFIG_QCOM_LLCC dependency") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/msm/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 52536e7adb95..69fbfe4568b2 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -9,14 +9,14 @@ config DRM_MSM depends on QCOM_OCMEM || QCOM_OCMEM=n depends on QCOM_LLCC || QCOM_LLCC=n depends on QCOM_COMMAND_DB || QCOM_COMMAND_DB=n + depends on QCOM_SCM || (QCOM_SCM=n && ARCH_QCOM=n) + depends on QCOM_MDT_LOADER || ARCH_QCOM=n select IOMMU_IO_PGTABLE - select QCOM_MDT_LOADER if ARCH_QCOM select REGULATOR select DRM_KMS_HELPER select DRM_PANEL select SHMEM select TMPFS - select QCOM_SCM if ARCH_QCOM select WANT_DEV_COREDUMP select SND_SOC_HDMI_CODEC if SND_SOC select SYNC_FILE -- 2.29.2
Re: [Freedreno] [PATCH 0/3] iommu/drm/msm: Allow non-coherent masters to use system cache
On Mon, Aug 2, 2021 at 3:55 AM Will Deacon wrote: > > On Thu, Jul 29, 2021 at 10:08:22AM +0530, Sai Prakash Ranjan wrote: > > On 2021-07-28 19:30, Georgi Djakov wrote: > > > On Mon, Jan 11, 2021 at 07:45:02PM +0530, Sai Prakash Ranjan wrote: > > > > commit ecd7274fb4cd ("iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag") > > > > removed unused IOMMU_SYS_CACHE_ONLY prot flag and along with it went > > > > the memory type setting required for the non-coherent masters to use > > > > system cache. Now that system cache support for GPU is added, we will > > > > need to set the right PTE attribute for GPU buffers to be sys cached. > > > > Without this, the system cache lines are not allocated for GPU. > > > > > > > > So the patches in this series introduces a new prot flag IOMMU_LLC, > > > > renames IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to IO_PGTABLE_QUIRK_PTW_LLC > > > > and makes GPU the user of this protection flag. > > > > > > Thank you for the patchset! Are you planning to refresh it, as it does > > > not apply anymore? > > > > > > > I was waiting on Will's reply [1]. If there are no changes needed, then > > I can repost the patch. > > I still think you need to handle the mismatched alias, no? You're adding > a new memory type to the SMMU which doesn't exist on the CPU side. That > can't be right. > Just curious, and maybe this is a dumb question, but what is your concern about mismatched aliases? I mean the cache hierarchy on the GPU device side (anything beyond the LLC) is pretty different and doesn't really care about the smmu pgtable attributes.. BR, -R
Re: [PATCH 3/4] drm/tiny: add simple-dbi driver
Hi Am 02.08.21 um 08:35 schrieb Icenowy Zheng: Add a driver for generic MIPI DBI panels initialized with MIPI DCS commands. Currently a ST7789V-based panel is added to it. This panel has its configuration pre-programmed into the controller, so no vendor-specific configuration is needed. Signed-off-by: Icenowy Zheng --- drivers/gpu/drm/tiny/Kconfig | 12 ++ drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/simple-dbi.c | 244 ++ 3 files changed, 257 insertions(+) create mode 100644 drivers/gpu/drm/tiny/simple-dbi.c diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index d31be274a2bd..6cfc57b68a46 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -144,6 +144,18 @@ config TINYDRM_REPAPER If M is selected the module will be called repaper. +config TINYDRM_SIMPLE_DBI + tristate "DRM support for generic MIPI DBI panels" + depends on DRM && SPI + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DRM_MIPI_DBI + help + DRM driver for generic DBI panels that are MIPI DCS compatible + and needs no vendor-specific setup commands. + + If M is selected the module will be called simple-dbi. + config TINYDRM_ST7586 tristate "DRM support for Sitronix ST7586 display panels" depends on DRM && SPI diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile index e09942895c77..2553de651aa3 100644 --- a/drivers/gpu/drm/tiny/Makefile +++ b/drivers/gpu/drm/tiny/Makefile @@ -13,3 +13,4 @@ obj-$(CONFIG_TINYDRM_MI0283QT)+= mi0283qt.o obj-$(CONFIG_TINYDRM_REPAPER) += repaper.o obj-$(CONFIG_TINYDRM_ST7586) += st7586.o obj-$(CONFIG_TINYDRM_ST7735R) += st7735r.o +obj-$(CONFIG_TINYDRM_SIMPLE_DBI) += simple-dbi.o diff --git a/drivers/gpu/drm/tiny/simple-dbi.c b/drivers/gpu/drm/tiny/simple-dbi.c new file mode 100644 index ..2b207e43d500 --- /dev/null +++ b/drivers/gpu/drm/tiny/simple-dbi.c @@ -0,0 +1,244 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * DRM driver for display panels with configuration preset and needs only + * standard MIPI DCS commands to bring up. + * + * Copyright (C) 2021 Sipeed + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#define MIPI_DCS_ADDRESS_MODE_BGR BIT(3) +#define MIPI_DCS_ADDRESS_MODE_REVERSE BIT(5) +#define MIPI_DCS_ADDRESS_MODE_RTL BIT(6) +#define MIPI_DCS_ADDRESS_MODE_BTT BIT(7) + +struct simple_dbi_cfg { + const struct drm_display_mode mode; + unsigned int left_offset; + unsigned int top_offset; + bool inverted; + bool write_only; + bool bgr; + bool right_to_left; + bool bottom_to_top; +}; + +struct simple_dbi_priv { + struct mipi_dbi_dev dbidev; /* Must be first for .release() */ Which release? + const struct simple_dbi_cfg *cfg; +}; + +static void simple_dbi_pipe_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) +{ + struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev); + struct simple_dbi_priv *priv = container_of(dbidev, + struct simple_dbi_priv, + dbidev); + struct mipi_dbi *dbi = &dbidev->dbi; + int ret, idx; + u8 addr_mode = 0x00; + + if (!drm_dev_enter(pipe->crtc.dev, &idx)) + return; + + DRM_DEBUG_KMS("\n"); I'm not a friend of such debugging statements. If you absolutely want to keep it, rather use drm_dbg_kms(). + + ret = mipi_dbi_poweron_reset(dbidev); + if (ret) + goto out_exit; + + mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE); + msleep(5); + + /* Currently tinydrm supports 16bit only now */ + mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT, +MIPI_DCS_PIXEL_FMT_16BIT); + + if (priv->cfg->inverted) + mipi_dbi_command(dbi, MIPI_DCS_ENTER_INVERT_MODE); + else + mipi_dbi_command(dbi, MIPI_DCS_EXIT_INVERT_MODE); + + if (priv->cfg->bgr) + addr_mode |= MIPI_DCS_ADDRESS_MODE_BGR; + + if (priv->cfg->right_to_left) + addr_mode |= MIPI_DCS_ADDRESS_MODE_RTL; + + if (priv->cfg->bottom_to_top) + addr_mode |= MIPI_DCS_ADDRESS_MODE_BTT; + + switch (dbidev->rotation) { + default: + break; + case 90: + addr_mode ^= MIPI_DCS_ADDRESS_MODE_REVERSE; + addr_mode ^= MIPI_DCS_ADDRESS_MODE_RTL; + break; + case 180: + addr
Re: [Freedreno] [PATCH 0/3] iommu/drm/msm: Allow non-coherent masters to use system cache
On Mon, Aug 02, 2021 at 08:08:07AM -0700, Rob Clark wrote: > On Mon, Aug 2, 2021 at 3:55 AM Will Deacon wrote: > > > > On Thu, Jul 29, 2021 at 10:08:22AM +0530, Sai Prakash Ranjan wrote: > > > On 2021-07-28 19:30, Georgi Djakov wrote: > > > > On Mon, Jan 11, 2021 at 07:45:02PM +0530, Sai Prakash Ranjan wrote: > > > > > commit ecd7274fb4cd ("iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag") > > > > > removed unused IOMMU_SYS_CACHE_ONLY prot flag and along with it went > > > > > the memory type setting required for the non-coherent masters to use > > > > > system cache. Now that system cache support for GPU is added, we will > > > > > need to set the right PTE attribute for GPU buffers to be sys cached. > > > > > Without this, the system cache lines are not allocated for GPU. > > > > > > > > > > So the patches in this series introduces a new prot flag IOMMU_LLC, > > > > > renames IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to IO_PGTABLE_QUIRK_PTW_LLC > > > > > and makes GPU the user of this protection flag. > > > > > > > > Thank you for the patchset! Are you planning to refresh it, as it does > > > > not apply anymore? > > > > > > > > > > I was waiting on Will's reply [1]. If there are no changes needed, then > > > I can repost the patch. > > > > I still think you need to handle the mismatched alias, no? You're adding > > a new memory type to the SMMU which doesn't exist on the CPU side. That > > can't be right. > > > > Just curious, and maybe this is a dumb question, but what is your > concern about mismatched aliases? I mean the cache hierarchy on the > GPU device side (anything beyond the LLC) is pretty different and > doesn't really care about the smmu pgtable attributes.. If the CPU accesses a shared buffer with different attributes to those which the device is using then you fall into the "mismatched memory attributes" part of the Arm architecture. It's reasonably unforgiving (you should go and read it) and in some cases can apply to speculative accesses as well, but the end result is typically loss of coherency. Will
linux-next: manual merge of the drm-intel tree with the drm-intel-fixes tree
Hi all, Today's linux-next merge of the drm-intel tree got a conflict in: drivers/gpu/drm/i915/intel_device_info.c between commit: 0f9ed3b2c9ec ("drm/i915/display/cnl+: Handle fused off DSC") from the drm-intel-fixes tree and commit: a4d082fc194a ("drm/i915: rename/remove CNL registers") from the drm-intel tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. diff --cc drivers/gpu/drm/i915/intel_device_info.c index e0a10f36acc1,305facedd284.. --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c
Re: [PATCH 09/14] drm/radeon: Convert to Linux IRQ interfaces
On Tue, Jul 27, 2021 at 2:27 PM Thomas Zimmermann wrote: > > Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's > IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers > don't benefit from using it. > > DRM IRQ callbacks are now being called directly or inlined. > > Signed-off-by: Thomas Zimmermann Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/radeon/radeon_drv.c | 4 --- > drivers/gpu/drm/radeon/radeon_irq_kms.c | 44 + > drivers/gpu/drm/radeon/radeon_kms.h | 4 --- > 3 files changed, 37 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c > b/drivers/gpu/drm/radeon/radeon_drv.c > index c8dd68152d65..b74cebca1f89 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -607,10 +607,6 @@ static const struct drm_driver kms_driver = { > .postclose = radeon_driver_postclose_kms, > .lastclose = radeon_driver_lastclose_kms, > .unload = radeon_driver_unload_kms, > - .irq_preinstall = radeon_driver_irq_preinstall_kms, > - .irq_postinstall = radeon_driver_irq_postinstall_kms, > - .irq_uninstall = radeon_driver_irq_uninstall_kms, > - .irq_handler = radeon_driver_irq_handler_kms, > .ioctls = radeon_ioctls_kms, > .num_ioctls = ARRAY_SIZE(radeon_ioctls_kms), > .dumb_create = radeon_mode_dumb_create, > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c > b/drivers/gpu/drm/radeon/radeon_irq_kms.c > index a36ce826d0c0..3907785d0798 100644 > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > @@ -31,7 +31,7 @@ > > #include > #include > -#include > +#include > #include > #include > #include > @@ -51,7 +51,7 @@ > * radeon_irq_process is a macro that points to the per-asic > * irq handler callback. > */ > -irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg) > +static irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg) > { > struct drm_device *dev = (struct drm_device *) arg; > struct radeon_device *rdev = dev->dev_private; > @@ -118,7 +118,7 @@ static void radeon_dp_work_func(struct work_struct *work) > * Gets the hw ready to enable irqs (all asics). > * This function disables all interrupt sources on the GPU. > */ > -void radeon_driver_irq_preinstall_kms(struct drm_device *dev) > +static void radeon_driver_irq_preinstall_kms(struct drm_device *dev) > { > struct radeon_device *rdev = dev->dev_private; > unsigned long irqflags; > @@ -150,7 +150,7 @@ void radeon_driver_irq_preinstall_kms(struct drm_device > *dev) > * Handles stuff to be done after enabling irqs (all asics). > * Returns 0 on success. > */ > -int radeon_driver_irq_postinstall_kms(struct drm_device *dev) > +static int radeon_driver_irq_postinstall_kms(struct drm_device *dev) > { > struct radeon_device *rdev = dev->dev_private; > > @@ -169,7 +169,7 @@ int radeon_driver_irq_postinstall_kms(struct drm_device > *dev) > * > * This function disables all interrupt sources on the GPU (all asics). > */ > -void radeon_driver_irq_uninstall_kms(struct drm_device *dev) > +static void radeon_driver_irq_uninstall_kms(struct drm_device *dev) > { > struct radeon_device *rdev = dev->dev_private; > unsigned long irqflags; > @@ -194,6 +194,36 @@ void radeon_driver_irq_uninstall_kms(struct drm_device > *dev) > spin_unlock_irqrestore(&rdev->irq.lock, irqflags); > } > > +static int radeon_irq_install(struct radeon_device *rdev, int irq) > +{ > + struct drm_device *dev = rdev->ddev; > + int ret; > + > + if (irq == IRQ_NOTCONNECTED) > + return -ENOTCONN; > + > + radeon_driver_irq_preinstall_kms(dev); > + > + /* PCI devices require shared interrupts. */ > + ret = request_irq(irq, radeon_driver_irq_handler_kms, > + IRQF_SHARED, dev->driver->name, dev); > + if (ret) > + return ret; > + > + radeon_driver_irq_postinstall_kms(dev); > + > + return 0; > +} > + > +static void radeon_irq_uninstall(struct radeon_device *rdev) > +{ > + struct drm_device *dev = rdev->ddev; > + struct pci_dev *pdev = to_pci_dev(dev->dev); > + > + radeon_driver_irq_uninstall_kms(dev); > + free_irq(pdev->irq, dev); > +} > + > /** > * radeon_msi_ok - asic specific msi checks > * > @@ -314,7 +344,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev) > INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); > > rdev->irq.installed = true; > - r = drm_irq_install(rdev->ddev, rdev->pdev->irq); > + r = radeon_irq_install(rdev, rdev->pdev->irq); > if (r) { > rdev->irq.installed = false; > flush_delayed_work(&rdev->hotplug_work); > @@ -335,7 +365,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev) > void radeon_irq_kms_fini(struct radeo
linux-next: manual merge of the drm-intel tree with Linus' tree
Hi all, Today's linux-next merge of the drm-intel tree got a conflict in: drivers/gpu/drm/i915/display/intel_display.c between commit: b4bde5554f70 ("drm/i915/display: split DISPLAY_VER 9 and 10 in intel_setup_outputs()") from Linus' tree and commits: cad83b405fe4 ("drm/i915/display: remove PORT_F workaround for CNL") ec387b8ff8d7 ("drm/i915/display: split DISPLAY_VER 9 and 10 in intel_setup_outputs()") from the drm-intel tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. diff --cc drivers/gpu/drm/i915/display/intel_display.c index 557871ee07db,3faedcb7ef42.. --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c
[PATCH 0/9] remove rcu support from i915_address_space
Hi all, Jason wanted to do that as part of the scheduler series, but I object since rcu is very, very hard to review when adding, and much, much harder even to review when removing. This is because simply looking for __rcu pointer annotations and rcu functions isn't enough, rcu is also relied upon in many datastructures which have internally and rcu_read_lock protection (or at least the required amount of barriers), like xarray. The other problem is that it inherits when chasing pointers, e.g. i915_gem_engines has an rcu pointer to intel_context, which has a non-rcu pointer to i915_address_space. But since we could look-up the entire chain under rcu i.e. engines->context[i]->vm this means more code to audit. The audit explodes pretty quickly. Anyway I'm reasonable confident I got them all in the current code, and slightly less confident that I managed to stitch together the full history. References to relevant commits throughout the series. Cheers, Daniel Daniel Vetter (9): drm/i915: Drop code to handle set-vm races from execbuf drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam drm/i915: Add i915_gem_context_is_full_ppgtt drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem drm/i915: Drop __rcu from gem_context->vm drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups drm/i915: Stop rcu support for i915_address_space drm/i915: Split out intel_context_create_user drivers/gpu/drm/i915/gem/i915_gem_context.c | 82 --- drivers/gpu/drm/i915/gem/i915_gem_context.h | 13 ++- .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 12 ++- .../gpu/drm/i915/gem/selftests/huge_pages.c | 8 +- .../drm/i915/gem/selftests/i915_gem_context.c | 32 +++- .../gpu/drm/i915/gem/selftests/mock_context.c | 2 +- drivers/gpu/drm/i915/gt/intel_context.c | 22 - drivers/gpu/drm/i915/gt/intel_context.h | 2 + drivers/gpu/drm/i915/gt/intel_ggtt.c | 1 - drivers/gpu/drm/i915/gt/intel_gtt.c | 6 +- drivers/gpu/drm/i915/gt/intel_gtt.h | 2 +- drivers/gpu/drm/i915/gt/selftest_execlists.c | 2 +- drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/i915_trace.h | 2 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 +- drivers/gpu/drm/i915/selftests/i915_vma.c | 4 +- 18 files changed, 79 insertions(+), 123 deletions(-) -- 2.32.0
[PATCH 1/9] drm/i915: Drop code to handle set-vm races from execbuf
Changing the vm from a finalized gem ctx is no longer possible, which means we don't have to check for that anymore. I was pondering whether to keep the check as a WARN_ON, but things go boom real bad real fast if the vm of a vma is wrong. Plus we'd need to also get the ggtt vm for !full-ppgtt platforms. Ditching it all seemed like a better idea. References: ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)") Signed-off-by: Daniel Vetter Cc: Jon Bloomfield Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Daniel Vetter Cc: "Thomas Hellström" Cc: Matthew Auld Cc: Lionel Landwerlin Cc: Dave Airlie Cc: Jason Ekstrand --- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 538d9d2e52b7..69e47b97d786 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -775,11 +775,7 @@ static int __eb_add_lut(struct i915_execbuffer *eb, /* Check that the context hasn't been closed in the meantime */ err = -EINTR; if (!mutex_lock_interruptible(&ctx->lut_mutex)) { - struct i915_address_space *vm = rcu_access_pointer(ctx->vm); - - if (unlikely(vm && vma->vm != vm)) - err = -EAGAIN; /* user racing with ctx set-vm */ - else if (likely(!i915_gem_context_is_closed(ctx))) + if (likely(!i915_gem_context_is_closed(ctx))) err = radix_tree_insert(&ctx->handles_vma, handle, vma); else err = -ENOENT; -- 2.32.0
[PATCH 2/9] drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm
The important part isn't so much that this does an rcu lookup - that's more an implementation detail, which will also be removed. The thing that makes this different from other functions is that it's gettting you the vm that batchbuffers will run in for that gem context, which is either a full ppgtt stored in gem->ctx, or the ggtt. We'll make more use of this function later on. Signed-off-by: Daniel Vetter Cc: Jon Bloomfield Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Daniel Vetter Cc: "Thomas Hellström" Cc: Matthew Auld Cc: Lionel Landwerlin Cc: Dave Airlie Cc: Jason Ekstrand --- drivers/gpu/drm/i915/gem/i915_gem_context.h | 2 +- drivers/gpu/drm/i915/gem/selftests/huge_pages.c | 4 ++-- drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 4 ++-- drivers/gpu/drm/i915/gt/selftest_execlists.c | 2 +- drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 2 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 ++-- drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h index 18060536b0c2..da6e8b506d96 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h @@ -155,7 +155,7 @@ i915_gem_context_vm(struct i915_gem_context *ctx) } static inline struct i915_address_space * -i915_gem_context_get_vm_rcu(struct i915_gem_context *ctx) +i915_gem_context_get_eb_vm(struct i915_gem_context *ctx) { struct i915_address_space *vm; diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c index a094f3ce1a90..6c68fe26bb32 100644 --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c @@ -1456,7 +1456,7 @@ static int igt_tmpfs_fallback(void *arg) struct i915_gem_context *ctx = arg; struct drm_i915_private *i915 = ctx->i915; struct vfsmount *gemfs = i915->mm.gemfs; - struct i915_address_space *vm = i915_gem_context_get_vm_rcu(ctx); + struct i915_address_space *vm = i915_gem_context_get_eb_vm(ctx); struct drm_i915_gem_object *obj; struct i915_vma *vma; u32 *vaddr; @@ -1512,7 +1512,7 @@ static int igt_shrink_thp(void *arg) { struct i915_gem_context *ctx = arg; struct drm_i915_private *i915 = ctx->i915; - struct i915_address_space *vm = i915_gem_context_get_vm_rcu(ctx); + struct i915_address_space *vm = i915_gem_context_get_eb_vm(ctx); struct drm_i915_gem_object *obj; struct i915_gem_engines_iter it; struct intel_context *ce; diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c index 8eb5050f8cb3..d436ce7fa25c 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c @@ -1528,7 +1528,7 @@ static int write_to_scratch(struct i915_gem_context *ctx, intel_gt_chipset_flush(engine->gt); - vm = i915_gem_context_get_vm_rcu(ctx); + vm = i915_gem_context_get_eb_vm(ctx); vma = i915_vma_instance(obj, vm, NULL); if (IS_ERR(vma)) { err = PTR_ERR(vma); @@ -1607,7 +1607,7 @@ static int read_from_scratch(struct i915_gem_context *ctx, if (GRAPHICS_VER(i915) >= 8) { const u32 GPR0 = engine->mmio_base + 0x600; - vm = i915_gem_context_get_vm_rcu(ctx); + vm = i915_gem_context_get_eb_vm(ctx); vma = i915_vma_instance(obj, vm, NULL); if (IS_ERR(vma)) { err = PTR_ERR(vma); diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c index f12ffe797639..b3863abc51f5 100644 --- a/drivers/gpu/drm/i915/gt/selftest_execlists.c +++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c @@ -3493,7 +3493,7 @@ static int smoke_submit(struct preempt_smoke *smoke, if (batch) { struct i915_address_space *vm; - vm = i915_gem_context_get_vm_rcu(ctx); + vm = i915_gem_context_get_eb_vm(ctx); vma = i915_vma_instance(batch, vm, NULL); i915_vm_put(vm); if (IS_ERR(vma)) diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c index 08f011f893b2..6023c418ee8a 100644 --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c @@ -117,7 +117,7 @@ static struct i915_request * hang_create_request(struct hang *h, struct intel_engine_cs *engine) { struct intel_gt *gt = h->gt; - struct i915_address_space *vm = i915_gem_context_get_vm_rcu(h->ctx); + struct i915_address_space *vm = i915_gem_context_get_eb_vm(h->ctx); st
[PATCH 3/9] drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam
Consolidates the "which is the vm my execbuf runs in" code a bit. We do some get/put which isn't really required, but all the other users want the refcounting, and I figured doing a function just for this getparam to avoid 2 atomis is a bit much. Signed-off-by: Daniel Vetter Cc: Jon Bloomfield Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Daniel Vetter Cc: "Thomas Hellström" Cc: Matthew Auld Cc: Lionel Landwerlin Cc: Dave Airlie Cc: Jason Ekstrand --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index cff72679ad7c..6263563e15d6 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -2124,6 +2124,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data, struct drm_i915_file_private *file_priv = file->driver_priv; struct drm_i915_gem_context_param *args = data; struct i915_gem_context *ctx; + struct i915_address_space *vm; int ret = 0; ctx = i915_gem_context_lookup(file_priv, args->ctx_id); @@ -2133,12 +2134,10 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data, switch (args->param) { case I915_CONTEXT_PARAM_GTT_SIZE: args->size = 0; - rcu_read_lock(); - if (rcu_access_pointer(ctx->vm)) - args->value = rcu_dereference(ctx->vm)->total; - else - args->value = to_i915(dev)->ggtt.vm.total; - rcu_read_unlock(); + vm = i915_gem_context_get_eb_vm(ctx); + args->value = vm->total; + i915_vm_put(vm); + break; case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE: -- 2.32.0
[PATCH 4/9] drm/i915: Add i915_gem_context_is_full_ppgtt
And use it anywhere we have open-coded checks for ctx->vm that really only check for full ppgtt. Plus for paranoia add a GEM_BUG_ON that checks it's really only set when we have full ppgtt, just in case. gem_context->vm is different since it's NULL in ggtt mode, unlike intel_context->vm or gt->vm, which is always set. Signed-off-by: Daniel Vetter Cc: Jon Bloomfield Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Daniel Vetter Cc: "Thomas Hellström" Cc: Matthew Auld Cc: Lionel Landwerlin Cc: Dave Airlie Cc: Jason Ekstrand --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_context.h | 7 +++ drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c| 2 +- drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 4 ++-- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 6263563e15d6..a80b06c98dba 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -1581,7 +1581,7 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv, int err; u32 id; - if (!rcu_access_pointer(ctx->vm)) + if (!i915_gem_context_is_full_ppgtt(ctx)) return -ENODEV; rcu_read_lock(); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h index da6e8b506d96..37536a260e6e 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h @@ -154,6 +154,13 @@ i915_gem_context_vm(struct i915_gem_context *ctx) return rcu_dereference_protected(ctx->vm, lockdep_is_held(&ctx->mutex)); } +static inline bool i915_gem_context_is_full_ppgtt(struct i915_gem_context *ctx) +{ + GEM_BUG_ON(!!rcu_access_pointer(ctx->vm) != HAS_FULL_PPGTT(ctx->i915)); + + return !!rcu_access_pointer(ctx->vm); +} + static inline struct i915_address_space * i915_gem_context_get_eb_vm(struct i915_gem_context *ctx) { diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 69e47b97d786..bdf2b5785a81 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -749,7 +749,7 @@ static int eb_select_context(struct i915_execbuffer *eb) return PTR_ERR(ctx); eb->gem_context = ctx; - if (rcu_access_pointer(ctx->vm)) + if (i915_gem_context_is_full_ppgtt(ctx)) eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT; return 0; diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c index d436ce7fa25c..5442b8e59629 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c @@ -838,7 +838,7 @@ static int igt_shared_ctx_exec(void *arg) pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) [full-ppgtt? %s], err=%d\n", ndwords, dw, max_dwords(obj), engine->name, - yesno(!!rcu_access_pointer(ctx->vm)), + yesno(i915_gem_context_is_full_ppgtt(ctx)), err); intel_context_put(ce); kernel_context_close(ctx); @@ -1417,7 +1417,7 @@ static int igt_ctx_readonly(void *arg) pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) [full-ppgtt? %s], err=%d\n", ndwords, dw, max_dwords(obj), ce->engine->name, - yesno(!!ctx_vm(ctx)), + yesno(i915_gem_context_is_full_ppgtt(ctx)), err); i915_gem_context_unlock_engines(ctx); goto out_file; -- 2.32.0
[PATCH 5/9] drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem
Since commit ccbc1b97948ab671335e950271e39766729736c3 Author: Jason Ekstrand Date: Thu Jul 8 10:48:30 2021 -0500 drm/i915/gem: Don't allow changing the VM on running contexts (v4) the gem_ctx->vm can't change anymore. Plus we always set the intel_context->vm, so might as well use the helper we have for that. This makes it very clear that we always overwrite intel_context->vm for userspace contexts, since the default is gt->vm, which is explicitly reserved for kernel context use. It would be good to split things up a bit further and avoid any possibility for an accident where we run kernel stuff in userspace vm or the other way round. Signed-off-by: Daniel Vetter Cc: Jon Bloomfield Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Daniel Vetter Cc: "Thomas Hellström" Cc: Matthew Auld Cc: Lionel Landwerlin Cc: Dave Airlie Cc: Jason Ekstrand --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 12 ++-- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index a80b06c98dba..fd24a1236682 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -784,16 +784,8 @@ static int intel_context_set_gem(struct intel_context *ce, ce->ring_size = SZ_16K; - if (rcu_access_pointer(ctx->vm)) { - struct i915_address_space *vm; - - rcu_read_lock(); - vm = context_get_vm_rcu(ctx); /* hmm */ - rcu_read_unlock(); - - i915_vm_put(ce->vm); - ce->vm = vm; - } + i915_vm_put(ce->vm); + ce->vm = i915_gem_context_get_eb_vm(ctx); if (ctx->sched.priority >= I915_PRIORITY_NORMAL && intel_engine_has_timeslices(ce->engine) && -- 2.32.0
[PATCH 7/9] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
We don't need the absolute speed of rcu for this. And i915_address_space in general dont need rcu protection anywhere else, after we've made gem contexts and engines a lot more immutable. Note that this semantically reverts commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499 Author: Chris Wilson Date: Fri Aug 30 19:03:25 2019 +0100 drm/i915: Use RCU for unlocked vm_idr lookup except we have the conversion from idr to xarray in between. Signed-off-by: Daniel Vetter Cc: Jon Bloomfield Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Daniel Vetter Cc: "Thomas Hellström" Cc: Matthew Auld Cc: Lionel Landwerlin Cc: Dave Airlie Cc: Jason Ekstrand --- drivers/gpu/drm/i915/i915_drv.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1488d166d91c..df2d723c894a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1880,11 +1880,11 @@ i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id) { struct i915_address_space *vm; - rcu_read_lock(); + xa_lock(&file_priv->vm_xa); vm = xa_load(&file_priv->vm_xa, id); if (vm && !kref_get_unless_zero(&vm->ref)) vm = NULL; - rcu_read_unlock(); + xa_unlock(&file_priv->vm_xa); return vm; } -- 2.32.0
[PATCH 6/9] drm/i915: Drop __rcu from gem_context->vm
It's been invariant since commit ccbc1b97948ab671335e950271e39766729736c3 Author: Jason Ekstrand Date: Thu Jul 8 10:48:30 2021 -0500 drm/i915/gem: Don't allow changing the VM on running contexts (v4) this just completes the deed. I've tried to split out prep work for more careful review as much as possible, this is what's left: - get_ppgtt gets simplified since we don't need to grab a temporary reference - we can rely on the temporary reference for the gem_ctx while we inspect the vm. The new vm_id still needs a full i915_vm_open ofc. This also removes the final caller of context_get_vm_rcu - A pile of selftests can now just look at ctx->vm instead of rcu_dereference_protected( , true) or similar things. - All callers of i915_gem_context_vm also disappear. - I've changed the hugepage selftest to set scrub_64K without any locking, because when we inspect that setting we're also not taking any locks either. It works because it's a selftests that's careful (single threaded gives you nice ordering) and not a live driver where races can happen from anywhere. These can only be split up further if we have some intermediate state with a bunch more rcu_dereference_protected(ctx->vm, true), just to shut up lockdep and sparse. The conversion to __rcu happened in commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1 Author: Chris Wilson Date: Fri Oct 4 14:40:09 2019 +0100 drm/i915: Move context management under GEM Note that we're not breaking the actual bugfix in there: The real bugfix is pushing the i915_vm_relase onto a separate worker, to avoid locking inversion issues. The rcu conversion was just thrown in for entertainment value on top (no vm lookup isn't even close to anything that's a hotpath where removing the single spinlock can be measured). Signed-off-by: Daniel Vetter Cc: Jon Bloomfield Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Daniel Vetter Cc: "Thomas Hellström" Cc: Matthew Auld Cc: Lionel Landwerlin Cc: Dave Airlie Cc: Jason Ekstrand --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 53 ++- drivers/gpu/drm/i915/gem/i915_gem_context.h | 14 ++--- .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 +- .../gpu/drm/i915/gem/selftests/huge_pages.c | 4 +- .../drm/i915/gem/selftests/i915_gem_context.c | 24 - drivers/gpu/drm/i915/i915_trace.h | 2 +- drivers/gpu/drm/i915/selftests/i915_vma.c | 2 +- 7 files changed, 21 insertions(+), 80 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index fd24a1236682..2f3cc73d4710 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -735,44 +735,6 @@ static int set_proto_ctx_param(struct drm_i915_file_private *fpriv, return ret; } -static struct i915_address_space * -context_get_vm_rcu(struct i915_gem_context *ctx) -{ - GEM_BUG_ON(!rcu_access_pointer(ctx->vm)); - - do { - struct i915_address_space *vm; - - /* -* We do not allow downgrading from full-ppgtt [to a shared -* global gtt], so ctx->vm cannot become NULL. -*/ - vm = rcu_dereference(ctx->vm); - if (!kref_get_unless_zero(&vm->ref)) - continue; - - /* -* This ppgtt may have be reallocated between -* the read and the kref, and reassigned to a third -* context. In order to avoid inadvertent sharing -* of this ppgtt with that third context (and not -* src), we have to confirm that we have the same -* ppgtt after passing through the strong memory -* barrier implied by a successful -* kref_get_unless_zero(). -* -* Once we have acquired the current ppgtt of ctx, -* we no longer care if it is released from ctx, as -* it cannot be reallocated elsewhere. -*/ - - if (vm == rcu_access_pointer(ctx->vm)) - return rcu_pointer_handoff(vm); - - i915_vm_put(vm); - } while (1); -} - static int intel_context_set_gem(struct intel_context *ce, struct i915_gem_context *ctx, struct intel_sseu sseu) @@ -1193,7 +1155,7 @@ static void context_close(struct i915_gem_context *ctx) set_closed_name(ctx); - vm = i915_gem_context_vm(ctx); + vm = ctx->vm; if (vm) i915_vm_close(vm); @@ -1350,7 +1312,7 @@ i915_gem_create_context(struct drm_i915_private *i915, vm = &ppgtt->vm; } if (vm) { - RCU_INIT_POINTER(ctx->vm, i915_vm_open(vm)); + ctx->vm = i915_vm_open(vm); /* i915_vm_open() tak
[PATCH 8/9] drm/i915: Stop rcu support for i915_address_space
The full audit is quite a bit of work: - i915_dpt has very simple lifetime (somehow we create a display pagetable vm per object, so its _very_ simple, there's only ever a single vma in there), and uses i915_vm_close(), which internally does a i915_vm_put(). No rcu. Aside: wtf is i915_dpt doing in the intel_display.c garbage collector as a new feature, instead of added as a separate file with some clean-ish interface. Also, i915_dpt unfortunately re-introduces some coding patterns from pre-dma_resv_lock conversion times. - i915_gem_proto_ctx is fully refcounted and no rcu, all protected by fpriv->proto_context_lock. - i915_gem_context is itself rcu protected, and that might leak to anything it points at. Before commit cf977e18610e66e48c31619e7e0cfa871be9eada Author: Chris Wilson Date: Wed Dec 2 11:21:40 2020 + drm/i915/gem: Spring clean debugfs and commit db80a1294c231b6ac725085f046bb2931e00c9db Author: Chris Wilson Date: Mon Jan 18 11:08:54 2021 + drm/i915/gem: Remove per-client stats from debugfs/i915_gem_objects we had a bunch of debugfs files that relied on rcu protecting everything, but those are gone now. The main one was removed even earlier with There doesn't seem to be anything left that's actually protecting stuff now that the ctx->vm itself is invariant. See commit ccbc1b97948ab671335e950271e39766729736c3 Author: Jason Ekstrand Date: Thu Jul 8 10:48:30 2021 -0500 drm/i915/gem: Don't allow changing the VM on running contexts (v4) Note that we drop the vm refcount before the final release of the gem context refcount, so this is all very dangerous even without rcu. Note that aside from later on creating new engines (a defunct feature) and debug output we're never looked at gem_ctx->vm for anything functional, hence why this is ok. Fingers crossed. Preceeding patches removed all vestiges of rcu use from gem_ctx->vm derferencing to make it clear it's really not used. The gem_ctx->rcu protection was introduced in commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1 Author: Chris Wilson Date: Fri Oct 4 14:40:09 2019 +0100 drm/i915: Move context management under GEM The commit message is somewhat entertaining because it fails to mention this fact completely, and compensates that by an in-commit changelog entry that claims that ctx->vm is protected by ctx->mutex. Which was the case _before_ this commit, but no longer after it. - intel_context holds a full reference. Unfortunately intel_context is also rcu protected and the reference to the ->vm is dropped before the rcu barrier - only the kfree is delayed. So again we need to check whether that leaks anywhere on the intel_context->vm. RCU is only used to protect intel_context sitting on the breadcrumb lists, which don't look at the vm anywhere, so we are fine. Nothing else relies on rcu protection of intel_context and hence is fully protected by the kref refcount alone, which protects intel_context->vm in turn. The breadcrumbs rcu usage was added in commit c744d50363b714783bbc88d986cc16def13710f7 Author: Chris Wilson Date: Thu Nov 26 14:04:06 2020 + drm/i915/gt: Split the breadcrumb spinlock between global and contexts its parent commit added the intel_context rcu protection: commit 14d1eaf08845c534963c83f754afe0cb14cb2512 Author: Chris Wilson Date: Thu Nov 26 14:04:05 2020 + drm/i915/gt: Protect context lifetime with RCU given some credence to my claim that I've actually caught them all. - drm_i915_gem_object's shares_resv_from pointer has a full refcount to the dma_resv, which is a sub-refcount that's released after the final i915_vm_put() has been called. Safe. Aside: Maybe we should have a struct dma_resv_shared which is just dma_resv + kref as a stand-alone thing. It's a pretty useful pattern which other drivers might want to copy. For a bit more context see commit 4d8151ae5329cf50781a02fd2298a909589a5bab Author: Thomas Hellström Date: Tue Jun 1 09:46:41 2021 +0200 drm/i915: Don't free shared locks while shared - the fpriv->vm_xa was relying on rcu_read_lock for lookup, but that was updated in a prep patch too to just be a spinlock-protected lookup. - intel_gt->vm is set at driver load in intel_gt_init() and released in intel_gt_driver_release(). There seems to be some issue that in some error paths this is called twice, but otherwise no rcu to be found anywhere. This was added in the below commit, which unfortunately doesn't explain why this complication exists. commit e6ba76480299a0d77c51d846f7467b1673aad25b Author: Chris Wilson Date: Sat Dec 21 16:03:24 2019 + drm/i915: Remove i915->kernel_cont
[PATCH 9/9] drm/i915: Split out intel_context_create_user
There's quite a fundamental difference between userspace contexts, and kernel contexts. Latter all share intel_gt->vm, former get their vm from gem_ctx->vm (on full ppgtt at least). By splitting context creation for userspace from kernel-internal ones we can make this all a bit more strict and WARN_ON if there's a vm already set in intel_context_set_gem(). All this is only possible because gem_ctx cannot chance their VM anymore since commit ccbc1b97948ab671335e950271e39766729736c3 Author: Jason Ekstrand Date: Thu Jul 8 10:48:30 2021 -0500 drm/i915/gem: Don't allow changing the VM on running contexts (v4) Signed-off-by: Daniel Vetter Cc: Jon Bloomfield Cc: Chris Wilson Cc: Maarten Lankhorst Cc: Joonas Lahtinen Cc: Daniel Vetter Cc: "Thomas Hellström" Cc: Matthew Auld Cc: Lionel Landwerlin Cc: Dave Airlie Cc: Jason Ekstrand --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 ++--- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 4 +++- .../gpu/drm/i915/gem/selftests/mock_context.c | 2 +- drivers/gpu/drm/i915/gt/intel_context.c | 22 +-- drivers/gpu/drm/i915/gt/intel_context.h | 2 ++ 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 2f3cc73d4710..13358e6749d9 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -746,7 +746,7 @@ static int intel_context_set_gem(struct intel_context *ce, ce->ring_size = SZ_16K; - i915_vm_put(ce->vm); + WARN_ON(ce->vm); ce->vm = i915_gem_context_get_eb_vm(ctx); if (ctx->sched.priority >= I915_PRIORITY_NORMAL && @@ -856,7 +856,7 @@ static struct i915_gem_engines *default_engines(struct i915_gem_context *ctx, GEM_BUG_ON(engine->legacy_idx >= I915_NUM_ENGINES); GEM_BUG_ON(e->engines[engine->legacy_idx]); - ce = intel_context_create(engine); + ce = intel_context_create_user(engine); if (IS_ERR(ce)) { err = ERR_CAST(ce); goto free_engines; @@ -897,7 +897,7 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx, switch (pe[n].type) { case I915_GEM_ENGINE_TYPE_PHYSICAL: - ce = intel_context_create(pe[n].engine); + ce = intel_context_create_user(pe[n].engine); break; case I915_GEM_ENGINE_TYPE_BALANCED: diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index bdf2b5785a81..54de94433365 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -30,15 +30,17 @@ struct eb_vma { struct i915_vma *vma; + struct drm_i915_gem_object *obj; unsigned int flags; + u32 handle; + /** This vma's place in the execbuf reservation list */ struct drm_i915_gem_exec_object2 *exec; struct list_head bind_link; struct list_head reloc_link; struct hlist_node node; - u32 handle; }; enum { diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c index fee070df1c97..e5efda1058a3 100644 --- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c +++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c @@ -124,7 +124,7 @@ live_context_for_engine(struct intel_engine_cs *engine, struct file *file) return ctx; } - ce = intel_context_create(engine); + ce = intel_context_create_user(engine); if (IS_ERR(ce)) { __free_engines(engines, 0); return ERR_CAST(ce); diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 745e84c72c90..9e33efb594dd 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -34,6 +34,23 @@ void intel_context_free(struct intel_context *ce) call_rcu(&ce->rcu, rcu_context_free); } +/* for user contexts, callers must set ce->vm correctly */ +struct intel_context * +intel_context_create_user(struct intel_engine_cs *engine) +{ + struct intel_context *ce; + + ce = intel_context_alloc(); + if (!ce) + return ERR_PTR(-ENOMEM); + + intel_context_init(ce, engine); + + trace_intel_context_create(ce); + return ce; +} + +/* for kernel-internal users only, sets ce->vm to intel_gt.vm */ struct intel_context * intel_context_create(struct intel_engine_cs *engine) { @@ -44,6 +61,8 @@ intel_context_create(struct intel_engine_cs *engine) return ERR_PTR(-ENOMEM); intel_context_init(ce, engine); + ce->vm = i915_vm_get(engine->gt->vm); + trace_
Re: [PATCH] drm/msm/dp: update is_connected status base on sink count at dp_pm_resume()
On 2021-07-30 11:57, Stephen Boyd wrote: Quoting Kuogee Hsieh (2021-07-28 14:30:54) Currently at dp_pm_resume() is_connected state is decided base on hpd connection status only. This will put is_connected in wrongly "true" state at the scenario that dongle attached to DUT but without hmdi cable connecting to it. Fix this problem by adding read sink count from dongle and decided is_connected state base on both sink count and hpd connection status. Please add a Fixes tag. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_display.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 2b660e9..9bcb261 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1308,6 +1308,17 @@ static int dp_display_remove(struct platform_device *pdev) return 0; } +static int dp_get_sink_count(struct dp_display_private *dp) +{ + u8 sink_count; + + sink_count = drm_dp_read_sink_count(dp->aux); drm_dp_read_sink_count() returns an int, not a u8. Comparing a u8 to less than zero doesn't make any sense as it isn't signed. + if (sink_count < 0) + return 0; + + return sink_count; +} We can drop this function and just have an int count in dp_pm_resume() that is compared to < 0 and then ignored. + static int dp_pm_resume(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); @@ -1327,14 +1338,22 @@ static int dp_pm_resume(struct device *dev) dp_catalog_ctrl_hpd_config(dp->catalog); - status = dp_catalog_link_is_connected(dp->catalog); + /* +* set sink to normal operation mode -- D0 +* before dpcd read +*/ + dp_link_psm_config(dp->link, &dp->panel->link_info, false); + if ((status = dp_catalog_link_is_connected(dp->catalog))) + dp->link->sink_count = dp_get_sink_count(dp); Do we need to call drm_dp_read_sink_count_cap() as well? no, we only need sink_count + else + dp->link->sink_count = 0; /* * can not declared display is connected unless * HDMI cable is plugged in and sink_count of * dongle become 1 */ - if (status && dp->link->sink_count) Is 'status' used anymore? If not, please remove it. Yes, it still used which used to decided to perform dpcd read sink count or not + if (dp->link->sink_count) dp->dp_display.is_connected = true; else dp->dp_display.is_connected = false;
Re: [PATCH v4 2/7] moduleparam: add data member to struct kernel_param
Hi Jim, On Sat, 31 Jul 2021 at 22:42, Jim Cromie wrote: > Use of this new data member will be rare, it might be worth redoing > this as a separate/sub-type to keep the base case. > > Signed-off-by: Jim Cromie > --- > include/linux/moduleparam.h | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h > index eed280fae433..e9495b1e794d 100644 > --- a/include/linux/moduleparam.h > +++ b/include/linux/moduleparam.h > @@ -78,6 +78,7 @@ struct kernel_param { > const struct kparam_string *str; > const struct kparam_array *arr; > }; > + void *data; Might as well make this "const void *" since it is a compile-time constant? -Emil
Re: [PATCH 42/64] net: qede: Use memset_after() for counters
On Mon, Aug 02, 2021 at 02:29:28PM +, Shai Malin wrote: > > On Tue, Jul 31, 2021 at 07:07:00PM -0300, Kees Cook wrote: > > On Tue, Jul 27, 2021 at 01:58:33PM -0700, Kees Cook wrote: > > > In preparation for FORTIFY_SOURCE performing compile-time and run-time > > > field bounds checking for memset(), avoid intentionally writing across > > > neighboring fields. > > > > > > Use memset_after() so memset() doesn't get confused about writing > > > beyond the destination member that is intended to be the starting point > > > of zeroing through the end of the struct. > > > > > > Signed-off-by: Kees Cook > > > --- > > > The old code seems to be doing the wrong thing: starting from not the > > > first member, but sized for the whole struct. Which is correct? > > > > Quick ping on this question. > > > > The old code seems to be doing the wrong thing: it starts from the second > > member and writes beyond int_info, clobbering qede_lock: > > Thanks for highlighting the problem, but actually, the memset is redundant. > We will remove it so the change will not be needed. > > > > > struct qede_dev { > > ... > > struct qed_int_info int_info; > > > > /* Smaller private variant of the RTNL lock */ > > struct mutexqede_lock; > > ... > > > > > > struct qed_int_info { > > struct msix_entry *msix; > > u8 msix_cnt; > > > > /* This should be updated by the protocol driver */ > > u8 used_cnt; > > }; > > > > Should this also clear the "msix" member, or should this not write > > beyond int_info? This patch does the latter. > > It should clear only the msix_cnt, no need to clear the entire > qed_int_info structure. Should used_cnt be cleared too? It is currently. Better yet, what patch do you suggest I replace this proposed one with? :) Thanks for looking at this! -Kees -- Kees Cook
Re: [Intel-gfx] [PATCH v4 3/7] dyndbg: add dyndbg-bitmap definer and callbacks
Hi Jim, On Sat, 31 Jul 2021 at 22:42, Jim Cromie wrote: > +struct dyndbg_bitdesc { > + /* bitpos is inferred from index in containing array */ > + char *prefix; > + char *help; AFAICT these two should also be constant, right? > +int param_set_dyndbg(const char *instr, const struct kernel_param *kp) > +{ > + unsigned int val; > + unsigned long changes, result; > + int rc, chgct = 0, totct = 0, bitpos, bitsmax; > + char query[OUR_QUERY_SIZE]; > + struct dyndbg_bitdesc *bitmap = (struct dyndbg_bitdesc *) kp->data; > + > + // pr_info("set_dyndbg: instr: %s curr: %d\n", instr, *kp->arg); Left-over debug code, here and below? -Emil
Re: [PATCH v4 5/7] i915/gvt: control pr_debug("gvt:")s with bits in parameters/debug_gvt
Hi Jim, On Sat, 31 Jul 2021 at 22:42, Jim Cromie wrote: > DYNDBG_BITMAP_DESC(__gvt_debug, "dyndbg bitmap desc", > { "gvt: cmd: ", "command processing" }, > { "gvt: core: ", "core help" }, > { "gvt: dpy: ", "display help" }, > { "gvt: el: ", "help" }, > { "gvt: irq: ", "help" }, > { "gvt: mm: ", "help" }, > { "gvt: mmio: ", "help" }, > { "gvt: render: ", "help" }, > { "gvt: sched: " "help" }); > Previous commit removed the space after the colon. The above example needs updating. This concludes a casual read-through on my end. Hope it helps o/ -Emil
Re: [RFC PATCH v3 1/6] drm/doc: Color Management and HDR10 RFC
Hi, Thanks for having a stab at this, it's a massive complex topic to solve. Do you have the the HTML rendered somewhere for convenience? On Fri, Jul 30, 2021 at 04:41:29PM -0400, Harry Wentland wrote: > Use the new DRM RFC doc section to capture the RFC previously only > described in the cover letter at > https://patchwork.freedesktop.org/series/89506/ > > v3: > * Add sections on single-plane and multi-plane HDR > * Describe approach to define HW details vs approach to define SW intentions > * Link Jeremy Cline's excellent HDR summaries > * Outline intention behind overly verbose doc > * Describe FP16 use-case > * Clean up links > > v2: create this doc > > v1: n/a > > Signed-off-by: Harry Wentland > --- > Documentation/gpu/rfc/color_intentions.drawio | 1 + > Documentation/gpu/rfc/color_intentions.svg| 3 + > Documentation/gpu/rfc/colorpipe | 1 + > Documentation/gpu/rfc/colorpipe.svg | 3 + > Documentation/gpu/rfc/hdr-wide-gamut.rst | 580 ++ > Documentation/gpu/rfc/index.rst | 1 + > 6 files changed, 589 insertions(+) > create mode 100644 Documentation/gpu/rfc/color_intentions.drawio > create mode 100644 Documentation/gpu/rfc/color_intentions.svg > create mode 100644 Documentation/gpu/rfc/colorpipe > create mode 100644 Documentation/gpu/rfc/colorpipe.svg > create mode 100644 Documentation/gpu/rfc/hdr-wide-gamut.rst > -- snip -- > + > +Mastering Luminances > + > + > +Even though we are able to describe the absolute luminance of a pixel > +using the PQ 2084 EOTF we are presented with physical limitations of the > +display technologies on the market today. Here are a few examples of > +luminance ranges of displays. > + > +.. flat-table:: > + :header-rows: 1 > + > + * - Display > + - Luminance range in nits > + > + * - Typical PC display > + - 0.3 - 200 > + > + * - Excellent LCD HDTV > + - 0.3 - 400 > + > + * - HDR LCD w/ local dimming > + - 0.05 - 1,500 > + > +Since no display can currently show the full 0.0005 to 10,000 nits > +luminance range of PQ the display will need to tone-map the HDR content, > +i.e to fit the content within a display's capabilities. To assist > +with tone-mapping HDR content is usually accompanied by a metadata > +that describes (among other things) the minimum and maximum mastering > +luminance, i.e. the maximum and minimum luminance of the display that > +was used to master the HDR content. > + > +The HDR metadata is currently defined on the drm_connector via the > +hdr_output_metadata blob property. > + > +It might be useful to define per-plane hdr metadata, as different planes > +might have been mastered differently. I think this only applies to the approach where all the processing is decided in the kernel right? If we directly expose each pipeline stage, and userspace controls everything, there's no need for the kernel to know the mastering luminance of any of the input content. The kernel would only need to know the eventual *output* luminance range, which might not even match any of the input content! ... > + > +How are we solving the problem? > +=== > + > +Single-plane > + > + > +If a single drm_plane is used no further work is required. The compositor > +will provide one HDR plane alongside a drm_connector's hdr_output_metadata > +and the display HW will output this plane without further processing if > +no CRTC LUTs are provided. > + > +If desired a compositor can use the CRTC LUTs for HDR content but without > +support for PWL or multi-segmented LUTs the quality of the operation is > +expected to be subpar for HDR content. > + > + > +Multi-plane > +--- > + > +In multi-plane configurations we need to solve the problem of blending > +HDR and SDR content. This blending should be done in linear space and > +therefore requires framebuffer data that is presented in linear space > +or a way to convert non-linear data to linear space. Additionally > +we need a way to define the luminance of any SDR content in relation > +to the HDR content. > + Android doesn't blend in linear space, so any API shouldn't be built around an assumption of linear blending. > +In order to present framebuffer data in linear space without losing a > +lot of precision it needs to be presented using 16 bpc precision. > + > + > +Defining HW Details > +--- > + > +One way to take full advantage of modern HW's color pipelines is by > +defining a "generic" pipeline that matches all capable HW. Something > +like this, which I took `from Uma Shankar`_ and expanded on: > + > +.. _from Uma Shankar: https://patchwork.freedesktop.org/series/90826/ > + > +.. kernel-figure:: colorpipe.svg I don't think this pipeline is expressive enough, in part because of Android's non-linear blending as I mentioned above, but also because the "tonemapping" block is a bit of a monolithic black-box. I'd be in favo
[BISECTED] 5.14.0-rc4 broke drm/ttm when !CONFIG_DEBUG_FS
Booting 5.14.0-rc4 on my box with Radeon graphics breaks with [drm:radeon_ttm_init [radeon]] *ERROR* failed initializing buffer object driver(-19). radeon :01:00.0: Fatal error during GPU init after which the screen goes black for the rest of kernel boot and early user-space init. Once the console login shows up the screen is in some legacy low-res mode and Xorg can't be started. A git bisect between v5.14-rc3 (good) and v5.14-rc4 (bad) identified # first bad commit: [69de4421bb4c103ef42a32bafc596e23918c106f] drm/ttm: Initialize debugfs from ttm_global_init() Reverting that from 5.14.0-rc4 gives me a working kernel again. Note that I have # CONFIG_DEBUG_FS is not set /Mikael
Re: [PATCH v4 2/7] moduleparam: add data member to struct kernel_param
On Mon, Aug 2, 2021 at 10:18 AM Emil Velikov wrote: > > Hi Jim, > > On Sat, 31 Jul 2021 at 22:42, Jim Cromie wrote: > > > Use of this new data member will be rare, it might be worth redoing > > this as a separate/sub-type to keep the base case. > > > > Signed-off-by: Jim Cromie > > --- > > include/linux/moduleparam.h | 11 +-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h > > index eed280fae433..e9495b1e794d 100644 > > --- a/include/linux/moduleparam.h > > +++ b/include/linux/moduleparam.h > > @@ -78,6 +78,7 @@ struct kernel_param { > > const struct kparam_string *str; > > const struct kparam_array *arr; > > }; > > + void *data; > > Might as well make this "const void *" since it is a compile-time constant? > yes indeed. revising. thanks > -Emil
Re: [Intel-gfx] [PATCH v4 3/7] dyndbg: add dyndbg-bitmap definer and callbacks
On Mon, Aug 2, 2021 at 10:24 AM Emil Velikov wrote: > > Hi Jim, > > On Sat, 31 Jul 2021 at 22:42, Jim Cromie wrote: > > > +struct dyndbg_bitdesc { > > + /* bitpos is inferred from index in containing array */ > > + char *prefix; > > + char *help; > AFAICT these two should also be constant, right? > > > > +int param_set_dyndbg(const char *instr, const struct kernel_param *kp) > > +{ > > + unsigned int val; > > + unsigned long changes, result; > > + int rc, chgct = 0, totct = 0, bitpos, bitsmax; > > + char query[OUR_QUERY_SIZE]; > > + struct dyndbg_bitdesc *bitmap = (struct dyndbg_bitdesc *) kp->data; > > + > > + // pr_info("set_dyndbg: instr: %s curr: %d\n", instr, *kp->arg); > Left-over debug code, here and below? yup, all fixed up locally, with a version that fully works. thanks. > > -Emil
Re: [PATCH] gpu/drm/amd: Remove duplicated include of drm_drv.h
On Mon, Aug 2, 2021 at 3:32 AM zhouchuangao wrote: > > Duplicate include header file > line 28: #include > line 44: #include > > Signed-off-by: zhouchuangao Applied. Thanks! Alex > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index 3ec5099..05f864f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -41,8 +41,6 @@ > #include "amdgpu_securedisplay.h" > #include "amdgpu_atomfirmware.h" > > -#include > - > static int psp_sysfs_init(struct amdgpu_device *adev); > static void psp_sysfs_fini(struct amdgpu_device *adev); > > -- > 2.7.4 >
Re: [BISECTED] 5.14.0-rc4 broke drm/ttm when !CONFIG_DEBUG_FS
[Not subscribed so please CC me. Manual quoting after using lore's in-reply-to functionality. First time doing that so hope I got it right.] Mikael Pettersson wrote... > Booting 5.14.0-rc4 on my box with Radeon graphics breaks with > > [drm:radeon_ttm_init [radeon]] *ERROR* failed initializing buffer > object driver(-19). > radeon :01:00.0: Fatal error during GPU init Seeing this here too. amdgpu on polaris-11, on an old amd-fx6100 system. > after which the screen goes black for the rest of kernel boot > and early user-space init. *NOT* seeing that. However, I have boot messages turned on by default and I see them as usual, only it stays in vga-console mode instead of switching to framebuffer after early-boot. I'm guessing MP has a high-res boot-splash which doesn't work in vga mode, thus the black-screen until the login shows up. > Once the console login shows up the screen is in some legacy low-res > mode and Xorg can't be started. > > A git bisect between v5.14-rc3 (good) and v5.14-rc4 (bad) identified > > # first bad commit: [69de4421bb4c103ef42a32bafc596e23918c106f] > drm/ttm: Initialize debugfs from ttm_global_init() > > Reverting that from 5.14.0-rc4 gives me a working kernel again. > > Note that I have > # CONFIG_DEBUG_FS is not set That all matches here, including the unset CONFIG_DEBUG_FS and confirming the revert on 5.14.0-rc4 works. -- Duncan - HTML messages treated as spam "They that can give up essential liberty to obtain a little temporary safety, deserve neither liberty nor safety." Benjamin Franklin