[git pull] drm fixes for 5.18-rc7
Hey Linus, Pretty quiet week on the fixes front, 4 amdgpu and one i915 fix. I think there might be a few misc fbdev ones outstanding, but I'll see if they are necessary and pass them on if so. Dave. drm-fixes-2022-05-13: drm fixes for 5.18-rc7 amdgpu: - Disable ASPM for VI boards on ADL platforms - S0ix DCN3.1 display fix - Resume regression fix - Stable pstate fix i915: - fix for kernel memory corruption when running a lot of OpenCL tests in parallel The following changes since commit c5eb0a61238dd6faf37f58c9ce61c9980aaffd7a: Linux 5.18-rc6 (2022-05-08 13:54:17 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-05-13 for you to fetch changes up to 5005e9814698f47c5a3698fcc56c9f5e6f1d4644: Merge tag 'amd-drm-fixes-5.18-2022-05-11' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes (2022-05-13 10:40:56 +1000) drm fixes for 5.18-rc7 amdgpu: - Disable ASPM for VI boards on ADL platforms - S0ix DCN3.1 display fix - Resume regression fix - Stable pstate fix i915: - fix for kernel memory corruption when running a lot of OpenCL tests in parallel Alex Deucher (2): Revert "drm/amd/pm: keep the BACO feature enabled for suspend" drm/amdgpu/ctx: only reset stable pstate if the user changed it (v2) Dave Airlie (2): Merge tag 'drm-intel-fixes-2022-05-12' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'amd-drm-fixes-5.18-2022-05-11' of https://gitlab.freedesktop.org/agd5f/linux into drm-fixes Eric Yang (1): drm/amd/display: undo clearing of z10 related function pointers Karol Herbst (1): drm/i915: Fix race in __i915_vma_remove_closed Richard Gong (1): drm/amdgpu: vi: disable ASPM on Intel Alder Lake based systems drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 5 + drivers/gpu/drm/amd/amdgpu/vi.c | 17 - drivers/gpu/drm/amd/display/dc/dcn31/dcn31_init.c | 5 - drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 8 +--- drivers/gpu/drm/i915/i915_vma.c | 11 +++ 5 files changed, 29 insertions(+), 17 deletions(-)
Re: Improve TTMs empty object handling
Patch set reviewed. Good stuff. Acked-by: Luben Tuikov Regards, Luben On 2022-05-09 09:09, Christian König wrote: > Hi everyone, > > re-sending this because Daniel was requesting a background why this is > useful. > > When TTM creates a buffer this object initially should not have any > backing store and there no resource object associated with it. The same > can happen when a driver requests that the backing store of an object is > destroyed without allocating a new one. > > This is really useful during initial buffer creation as well as temporary > buffers and page tables which content doesn't need to be preserved when > they are evicted. > > Currently TTM allocates dummy system resources for that because drivers > couldn't handle a NULL pointer there. Audit the drivers and then clean > up TTM to stop making those dummy allocations. > > Please review and comment, > Christian. > >
[PATCH v2 8/8] dt-bindings: arm: qcom: document sda660 SoC and ifc6560 board
Add binding documentation for the Inforce IFC6560 board which uses Snapdragon SDA660. Signed-off-by: Dmitry Baryshkov --- Documentation/devicetree/bindings/arm/qcom.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml index 129cdd246223..ac4ee0f874ea 100644 --- a/Documentation/devicetree/bindings/arm/qcom.yaml +++ b/Documentation/devicetree/bindings/arm/qcom.yaml @@ -41,6 +41,7 @@ description: | sa8155p sc7180 sc7280 +sda660 sdm630 sdm632 sdm660 @@ -225,6 +226,11 @@ properties: - google,senor - const: qcom,sc7280 + - items: + - enum: + - inforce,ifc6560 + - const: qcom,sda660 + - items: - enum: - fairphone,fp3 -- 2.35.1
[PATCH v2 6/8] arm64: dts: qcom: sdm630: use defined symbols for interconnects
Replace numeric values with the symbolic names defined in the bindings
header.
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sdm630.dtsi | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi
b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index 17a1877587cf..01a1a1703568 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1045,7 +1046,7 @@ adreno_gpu: gpu@500 {
nvmem-cells = <_speed_bin>;
nvmem-cell-names = "speed_bin";
- interconnects = < 1 5>;
+ interconnects = < MASTER_APSS_PROC
SLAVE_EBI>;
interconnect-names = "gfx-mem";
operating-points-v2 = <_sdm630_opp_table>;
@@ -1299,8 +1300,8 @@ sdhc_2: sdhci@c084000 {
<_board>;
clock-names = "core", "iface", "xo";
- interconnects = < 3 10>,
- < 0 28>;
+ interconnects = < MASTER_SDCC_2
SLAVE_A2NOC_SNOC>,
+ < MASTER_APSS_PROC
SLAVE_SDCC_2>;
operating-points-v2 = <_opp_table>;
pinctrl-names = "default", "sleep";
@@ -1351,8 +1352,8 @@ sdhc_1: sdhci@c0c4000 {
< GCC_SDCC1_ICE_CORE_CLK>;
clock-names = "core", "iface", "xo", "ice";
- interconnects = < 2 10>,
- < 0 27>;
+ interconnects = < MASTER_SDCC_1
SLAVE_A2NOC_SNOC>,
+ < MASTER_APSS_PROC
SLAVE_SDCC_1>;
interconnect-names = "sdhc1-ddr", "cpu-sdhc1";
operating-points-v2 = <_opp_table>;
pinctrl-names = "default", "sleep";
@@ -1525,9 +1526,9 @@ mdp: mdp@c901000 {
"core",
"vsync";
- interconnects = < 2 5>,
- < 3 5>,
- < 0 17>;
+ interconnects = < MASTER_MDP_P0
SLAVE_EBI>,
+ < MASTER_MDP_P1
SLAVE_EBI>,
+ < MASTER_APSS_PROC
SLAVE_DISPLAY_CFG>;
interconnect-names = "mdp0-mem",
"mdp1-mem",
"rotator-mem";
@@ -2034,7 +2035,7 @@ camss: camss@ca0 {
"cphy_csid1",
"cphy_csid2",
"cphy_csid3";
- interconnects = < 5 5>;
+ interconnects = < MASTER_VFE SLAVE_EBI>;
interconnect-names = "vfe-mem";
iommus = <_smmu 0xc00>,
<_smmu 0xc01>,
@@ -2097,8 +2098,8 @@ venus: video-codec@cc0 {
< VIDEO_AXI_CLK>,
< THROTTLE_VIDEO_AXI_CLK>;
clock-names = "core", "iface", "bus", "bus_throttle";
- interconnects = < 0 13>,
- < 4 5>;
+ interconnects = < MASTER_APSS_PROC
SLAVE_VENUS_CFG>,
+ < MASTER_VENUS SLAVE_EBI>;
interconnect-names = "cpu-cfg", "video-mem";
interrupts = ;
iommus = <_smmu 0x400>,
--
2.35.1
[PATCH v2 5/8] arm64: dts: qcom: sdm630: add second (HS) USB host support
Add DT entries for the second DWC3 USB host, which is limited to the
USB2.0 (HighSpeed), and the corresponding QUSB PHY.
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sdm630.dtsi | 55
1 file changed, 55 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi
b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index cca56f2fad96..17a1877587cf 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -1270,6 +1270,20 @@ qusb2phy: phy@c012000 {
status = "disabled";
};
+ qusb2phy1: phy@c014000 {
+ compatible = "qcom,sdm660-qusb2-phy";
+ reg = <0x0c014000 0x180>;
+ #phy-cells = <0>;
+
+ clocks = < GCC_USB_PHY_CFG_AHB2PHY_CLK>,
+ < GCC_RX1_USB2_CLKREF_CLK>;
+ clock-names = "cfg_ahb", "ref";
+
+ resets = < GCC_QUSB2PHY_SEC_BCR>;
+ nvmem-cells = <_hstx_trim>;
+ status = "disabled";
+ };
+
sdhc_2: sdhci@c084000 {
compatible = "qcom,sdm630-sdhci", "qcom,sdhci-msm-v5";
reg = <0x0c084000 0x1000>;
@@ -1375,6 +1389,47 @@ opp-38400 {
};
};
+ usb2: usb@c2f8800 {
+ compatible = "qcom,sdm660-dwc3", "qcom,dwc3";
+ reg = <0x0c2f8800 0x400>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ clocks = < GCC_CFG_NOC_USB2_AXI_CLK>,
+< GCC_USB20_MASTER_CLK>,
+< GCC_USB20_MOCK_UTMI_CLK>,
+< GCC_USB20_SLEEP_CLK>;
+ clock-names = "cfg_noc", "core",
+ "mock_utmi", "sleep";
+
+ assigned-clocks = < GCC_USB20_MOCK_UTMI_CLK>,
+ < GCC_USB20_MASTER_CLK>;
+ assigned-clock-rates = <1920>, <6000>;
+
+ interrupts = ;
+ interrupt-names = "hs_phy_irq";
+
+ qcom,select-utmi-as-pipe-clk;
+
+ resets = < GCC_USB_20_BCR>;
+
+ usb2_dwc3: usb@c20 {
+ compatible = "snps,dwc3";
+ reg = <0x0c20 0xc8d0>;
+ interrupts = ;
+ snps,dis_u2_susphy_quirk;
+ snps,dis_enblslpm_quirk;
+
+ /* This is the HS-only host */
+ maximum-speed = "high-speed";
+ phys = <>;
+ phy-names = "usb2-phy";
+ snps,hird-threshold = /bits/ 8 <0>;
+ };
+ };
+
mmcc: clock-controller@c8c {
compatible = "qcom,mmcc-sdm630";
reg = <0x0c8c 0x4>;
--
2.35.1
[PATCH v2 7/8] arm64: dts: qcom: sdm660: Add initial Inforce IFC6560 board support
The IFC6560 is a board from Inforce Computing, built around the SDA660
SoC. This patch describes core clocks, some regulators from the two
PMICs, debug uart, storage, bluetooth and audio DSP remoteproc.
The regulator settings are inherited from prior work by Konrad Dybcio
and AngeloGioacchino Del Regno.
Co-developed-by: Bjorn Andersson
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../boot/dts/qcom/sda660-inforce-ifc6560.dts | 459 ++
2 files changed, 460 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sda660-inforce-ifc6560.dts
diff --git a/arch/arm64/boot/dts/qcom/Makefile
b/arch/arm64/boot/dts/qcom/Makefile
index f9e6343acd03..5f717fe0e8d0 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -88,6 +88,7 @@ dtb-$(CONFIG_ARCH_QCOM) +=
sc7280-herobrine-herobrine-r1.dtb
dtb-$(CONFIG_ARCH_QCOM)+= sc7280-idp.dtb
dtb-$(CONFIG_ARCH_QCOM)+= sc7280-idp2.dtb
dtb-$(CONFIG_ARCH_QCOM)+= sc7280-crd.dtb
+dtb-$(CONFIG_ARCH_QCOM)+= sda660-inforce-ifc6560.dtb
dtb-$(CONFIG_ARCH_QCOM)+= sdm630-sony-xperia-ganges-kirin.dtb
dtb-$(CONFIG_ARCH_QCOM)+= sdm630-sony-xperia-nile-discovery.dtb
dtb-$(CONFIG_ARCH_QCOM)+= sdm630-sony-xperia-nile-pioneer.dtb
diff --git a/arch/arm64/boot/dts/qcom/sda660-inforce-ifc6560.dts
b/arch/arm64/boot/dts/qcom/sda660-inforce-ifc6560.dts
new file mode 100644
index ..d4b80ee4caf8
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sda660-inforce-ifc6560.dts
@@ -0,0 +1,459 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2021, Linaro Ltd.
+ * Copyright (c) 2020, Konrad Dybcio
+ * Copyright (c) 2020, AngeloGioacchino Del Regno
+ *
+ */
+
+/dts-v1/;
+
+#include "sdm660.dtsi"
+#include "pm660.dtsi"
+#include "pm660l.dtsi"
+
+/ {
+ model = "Inforce 6560 Single Board Computer";
+ compatible = "inforce,ifc6560", "qcom,sda660";
+ chassis-type = "embedded"; /* SBC */
+
+ aliases {
+ serial0 = _uart2;
+ serial1 = _uart1;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ volup {
+ label = "Volume Up";
+ gpios = <_gpios 7 GPIO_ACTIVE_LOW>;
+ linux,code = ;
+ debounce-interval = <15>;
+ };
+ };
+
+ /*
+* Until we hook up type-c detection, we
+* have to stick with this. But it works.
+*/
+ extcon_usb: extcon-usb {
+ compatible = "linux,extcon-usb-gpio";
+ id-gpio = < 58 GPIO_ACTIVE_HIGH>;
+ };
+
+ hdmi-out {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_con: endpoint {
+ remote-endpoint = <_out>;
+ };
+ };
+ };
+
+ vph_pwr: vph-pwr-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "vph_pwr";
+ regulator-min-microvolt = <380>;
+ regulator-max-microvolt = <380>;
+
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ v3p3_bck_bst: v3p3-bck-bst-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "v3p3_bck_bst";
+
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+
+ vin-supply = <_pwr>;
+ };
+
+ v1p2_ldo: v1p2-ldo-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "v1p2_ldo";
+
+ regulator-min-microvolt = <120>;
+ regulator-max-microvolt = <120>;
+
+ vin-supply = <_pwr>;
+ };
+
+ v5p0_boost: v5p0-boost-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "v5p0_boost";
+
+ regulator-min-microvolt = <500>;
+ regulator-max-microvolt = <500>;
+
+ vin-supply = <_pwr>;
+ };
+};
+
+_pil {
+ firmware-name = "qcom/ifc6560/adsp.mbn";
+};
+
+_dma {
+ /*
+* The board will lock up if we toggle the BLSP clock, unless the
+* BAM DMA interconnects support is in place.
+*/
+ /delete-property/ clocks;
+};
+
+_i2c6 {
+ status = "okay";
+
+ adv7533: hdmi@39 {
+ compatible = "adi,adv7535";
+ reg = <0x39>, <0x66>;
+ reg-names = "main", "edid";
+
+ interrupt-parent = <_gpios>;
+ interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
+
+ clocks = < RPM_SMD_BB_CLK2>;
+ clock-names = "cec";
+ /*
+* Limit to
[PATCH v2 1/8] arm64: dts: qcom: sdm660: disable dsi1/dsi1_phy by default
Follow the typical practice and keep DSI1/DSI1 PHY disabled by default.
They should be enabled in the board DT files. No existing boards use
them at this moment.
Reviewed-by: Marijn Suijten
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sdm660.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm660.dtsi
b/arch/arm64/boot/dts/qcom/sdm660.dtsi
index eccf6fde16b4..023b0ac4118c 100644
--- a/arch/arm64/boot/dts/qcom/sdm660.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm660.dtsi
@@ -192,6 +192,8 @@ dsi1: dsi@c996000 {
phys = <_phy>;
phy-names = "dsi";
+ status = "disabled";
+
ports {
#address-cells = <1>;
#size-cells = <0>;
@@ -225,6 +227,7 @@ dsi1_phy: dsi-phy@c996400 {
clocks = < MDSS_AHB_CLK>, < RPM_SMD_XO_CLK_SRC>;
clock-names = "iface", "ref";
+ status = "disabled";
};
};
--
2.35.1
[PATCH v2 2/8] arm64: dts: qcom: sdm630: disable dsi1/dsi1_phy by default
Follow the typical practice and keep DSI0/DSI0 PHY disabled by default.
They should be enabled in the board DT files. No existing boards use
them at this moment.
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sdm630.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi
b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index 240293592ef9..8697d40e9b74 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -1559,6 +1559,8 @@ dsi0: dsi@c994000 {
phys = <_phy>;
phy-names = "dsi";
+ status = "disabled";
+
ports {
#address-cells = <1>;
#size-cells = <0>;
@@ -1592,6 +1594,7 @@ dsi0_phy: dsi-phy@c994400 {
clocks = < MDSS_AHB_CLK>, <_board>;
clock-names = "iface", "ref";
+ status = "disabled";
};
};
--
2.35.1
[PATCH v2 3/8] arm64: dts: qcom: sdm630: disable GPU by default
The SoC's device tree file disables gpucc and adreno's SMMU by default.
So let's disable the GPU too. Moreover it looks like SMMU might be not
usable without additional patches (which means that GPU is unusable
too). No board uses GPU at this moment.
Fixes: 5cf69dcbec8b ("arm64: dts: qcom: sdm630: Add Adreno 508 GPU
configuration")
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sdm630.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi
b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index 8697d40e9b74..e8bb170e8b2f 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -1050,6 +1050,8 @@ adreno_gpu: gpu@500 {
operating-points-v2 = <_sdm630_opp_table>;
+ status = "disabled";
+
gpu_sdm630_opp_table: opp-table {
compatible = "operating-points-v2";
opp-77500 {
--
2.35.1
[PATCH v2 4/8] arm64: dts: qcom: sdm630: fix the qusb2phy ref clock
According to the downstram DT file, the qusb2phy ref clock should be
GCC_RX0_USB2_CLKREF_CLK, not GCC_RX1_USB2_CLKREF_CLK.
Fixes: c65a4ed2ea8b ("arm64: dts: qcom: sdm630: Add USB configuration")
Cc: Konrad Dybcio
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sdm630.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi
b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index e8bb170e8b2f..cca56f2fad96 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -1262,7 +1262,7 @@ qusb2phy: phy@c012000 {
#phy-cells = <0>;
clocks = < GCC_USB_PHY_CFG_AHB2PHY_CLK>,
- < GCC_RX1_USB2_CLKREF_CLK>;
+ < GCC_RX0_USB2_CLKREF_CLK>;
clock-names = "cfg_ahb", "ref";
resets = < GCC_QUSB2PHY_PRIM_BCR>;
--
2.35.1
[PATCH v2 0/8] arm64: dts: qcom: initial Inforce IFC6560 board support
This work is largely based on the previous work by Bjorn Andersson ([1]) Changes since v1 (mostly based on Kondrad's review): - Also disabled dsi0/dsi0 phy in sdm630.dtsi - Removed the clock from BAM DMA devices rather than disabling them completely - Replaced numbers with symbolic names for interconnects in sdm630.dtsi - Switched to "qcom,sda660" as a fallback compatible string - Added dt-bindings for the qcom,sda660 compat - Removed extra nesting level from the adsp firmware path - Replaced numbers with proper symbolic names in the board file - Added chassis-type property - Changed the order of blsp entries in the board file - Removed spurious newlines - Changed the order of regulator properties - Changed the DSI data-lines to list all four lanes. Still use just three lanes for the adv bridge (and describe the reason in the comment) Changes since Bjorn's v2: - Disable dsi1, dsi1 phy, GPU by default in sdm660.dtsi/sdm630.dtsi - Fix qusb2phy ref clock - Added USB2 host support to sdm630.dtsi - Renamed DTS to follow SoC-vendor-board pattern - Fixed vph_pwr voltage - Removed extra/unrelated comments - Added keys, USB2, USB3, - Added configuration for the attached HDMI bridge - Enabled MDP, MDSS and DSI0/DSI0 PHY devices - Removed uart pinctrl and /reserved-mem nodes (present in main dtsi file) - Added card detection for the SDCC2 - Disabled BLSP BAM DMA devices, they make the board reset during boot [1] https://lore.kernel.org/linux-arm-msm/20210825221110.1498718-1-bjorn.anders...@linaro.org/#t Dmitry Baryshkov (8): arm64: dts: qcom: sdm660: disable dsi1/dsi1_phy by default arm64: dts: qcom: sdm630: disable dsi1/dsi1_phy by default arm64: dts: qcom: sdm630: disable GPU by default arm64: dts: qcom: sdm630: fix the qusb2phy ref clock arm64: dts: qcom: sdm630: add second (HS) USB host support arm64: dts: qcom: sdm630: use defined symbols for interconnects arm64: dts: qcom: sdm660: Add initial Inforce IFC6560 board support dt-bindings: arm: qcom: document sda660 SoC and ifc6560 board .../devicetree/bindings/arm/qcom.yaml | 6 + arch/arm64/boot/dts/qcom/Makefile | 1 + .../boot/dts/qcom/sda660-inforce-ifc6560.dts | 459 ++ arch/arm64/boot/dts/qcom/sdm630.dtsi | 85 +++- arch/arm64/boot/dts/qcom/sdm660.dtsi | 3 + 5 files changed, 542 insertions(+), 12 deletions(-) create mode 100644 arch/arm64/boot/dts/qcom/sda660-inforce-ifc6560.dts -- 2.35.1
Re: [PATCH v3 1/4] drm/dp: Add wait_hpd_asserted() callback to struct drm_dp_aux
Hi,
On Wed, May 11, 2022 at 6:58 PM Stephen Boyd wrote:
>
> Quoting Douglas Anderson (2022-04-18 10:17:54)
> > Sometimes it's useful for users of the DP AUX bus (like panels) to be
> > able to poll HPD. Let's add a callback that allows DP AUX busses
> > drivers to provide this.
> >
> > Suggested-by: Dmitry Baryshkov
> > Signed-off-by: Douglas Anderson
> > ---
> > Left Dmitry's Reviewed-by tag off since patch changed enough.
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - Change is_hpd_asserted() to wait_hpd_asserted()
> >
> > include/drm/dp/drm_dp_helper.h | 26 ++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/include/drm/dp/drm_dp_helper.h b/include/drm/dp/drm_dp_helper.h
> > index 53d1e722f4de..0940c415db8c 100644
> > --- a/include/drm/dp/drm_dp_helper.h
> > +++ b/include/drm/dp/drm_dp_helper.h
> > @@ -2035,6 +2035,32 @@ struct drm_dp_aux {
> > ssize_t (*transfer)(struct drm_dp_aux *aux,
> > struct drm_dp_aux_msg *msg);
> >
> > + /**
> > +* @wait_hpd_asserted: wait for HPD to be asserted
> > +*
> > +* This is mainly useful for eDP panels drivers to wait for an eDP
> > +* panel to finish powering on. This is an optional function.
>
> Is there any use for the opposite direction? For example, does anything
> care that HPD is deasserted?
Not that I'm aware of. Originally I was planning to have it so that a
timeout of "0" meant to just poll without sleeping at all, but it
ended up making the code a lot more complicated because everywhere
else we had the "readx" semantics where 0 meant wait forever. It
didn't seem worth it. I can go back to that behavior if need be.
> > +*
> > +* This function will efficiently wait for up to `wait_us`
> > microseconds
> > +* for HPD to be asserted and might sleep.
> > +*
> > +* This function returns 0 if HPD was asserted or -ETIMEDOUT if time
> > +* expired and HPD wasn't asserted. This function should not print
> > +* timeout errors to the log.
> > +*
> > +* The semantics of this function are designed to match the
> > +* readx_poll_timeout() function. That means a `wait_us` of 0 means
> > +* to wait forever. If you want to do a quick poll you could pass 1
> > +* for `wait_us`.
>
> It would also make sense to have a drm_dp_wait_hpd_asserted() API
>
> int drm_dp_wait_hpd_asserted(struct drm_dp_aux *aux, unsigned long wait_us);
>
> and then this aux function could be implemented in various ways. The API
> could poll if the aux can only read immediate state of HPD, or it could
> sleep (is sleeping allowed? that isn't clear) and wake up the process
> once HPD goes high. Or if this op isn't implemented maybe there's a
> fixed timeout member that is non-zero which means "sleep this long".
> Either way, making each drm_dp_aux implement that logic seems error
> prone vs. having the drm_dp_aux implement some function for
>
> get_immediate_hpd(struct drm_dp_aux *aux)
There's a reason why I changed the API to "wait" from "get". If you
can think of a good place to document this, I'm all ears.
The basic problem is ps8640 (my nemesis, apparently). On ps8640,
because of the black box firmware blob that's on it, we have a crazy
long delay in its runtime resume (300ms). So what happens with ps8640
is that if we make the API "get_immediate_hpd()" it wasn't so
immediate. Even with autosuspend, that first "get" could take 300 ms,
which really screwed with everyone else who was waiting with a 200 ms
timeout.
Now, in theory, one could argue that the fact that ps8640 had a 300 ms
sleep would mean that the very first "get" of the panel would already
show HPD high. I don't know why that wasn't the case, but ps8640 is an
annoying black box.
In general, though, the DP controller might need some amount of time
to power itself back up and configure itself. Even though the ps8640
case is extreme, it wouldn't be totally extreme to assume that an AUX
controller might take 20 ms or 50 ms to power up. That could still
throw timings off. Implementing the API as a "wait" style API gets
around this problem. Now the DP controller can take as long as it
needs to power itself up and it can then wait with the requested
timeout.
> or
>
> notify_on_hpd(struct drm_dp_aux *auxstruct completion *comp)
>
> > +*
> > +* NOTE: this function specifically reports the state of the HPD pin
> > +* that's associated with the DP AUX channel. This is different from
> > +* the HPD concept in much of the rest of DRM which is more about
> > +* physical presence of a display. For eDP, for instance, a display
> > is
> > +* assumed always present even if the HPD pin is deasserted.
> > +*/
> > + int (*wait_hpd_asserted)(struct drm_dp_aux *aux, unsigned long
> > wait_us);
> > +
> > /**
> > * @i2c_nack_count: Counts
Re: [PATCH v4] drm/msm/dsi: don't powerup at modeset time for parade-ps8640
Hi,
On 5/12/2022 3:44 PM, Doug Anderson wrote:
Hi,
On Thu, May 12, 2022 at 3:34 PM Abhinav Kumar wrote:
On 5/12/2022 3:16 PM, Dmitry Baryshkov wrote:
On 13/05/2022 01:00, Douglas Anderson wrote:
Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
time") caused sc7180 Chromebooks that use the parade-ps8640 bridge
chip to fail to turn the display back on after it turns off.
Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to
handle the new power sequence. The Linux driver has almost nothing in
it and most of the logic for this bridge chip is in black-box firmware
that the bridge chip uses.
Also unfortunately, reverting the patch will break "tc358762".
The long term solution here is probably Dave Stevenson's series [1]
that would give more flexibility. However, that is likely not a quick
fix.
For the short term, we'll look at the compatible of the next bridge in
the chain and go back to the old way for the Parade PS8640 bridge
chip. If it's found that other bridge chips also need this workaround
then we can add them to the list or consider inverting the condition.
[1]
https://lore.kernel.org/r/cover.1646406653.git.dave.steven...@raspberrypi.com
Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
time")
Suggested-by: Rob Clark
Signed-off-by: Douglas Anderson
Reviewed-by: Dmitry Baryshkov
Yes, I think this is a better solution than a full revert
Reviewed-by: Abhinav Kumar
I am curious to know why this doesnt work for parade but will not hold
this patch back for that. We are initializing and turning on DSI PHY now
before turning on the bridge chip which is actually better as we are
putting PHY in a good state.
So this should have been better, but somehow doesn't work.
I can't really explain it, but mostly because the Parade chip is just
a big black box. There have been several times when an OEM using this
bridge chip had one problem or another with getting the display to
turn on, then the parade FAE would make some magic tweak to the
firmware and it would be fixed. The current way that the Linux driver
is working is with pretty much zero configuration so I think this chip
bakes in a bunch of assumptions about the timings / signal coming from
the MIPI DSI side. It doesn't surprise me that changing the order like
this would confuse it.
In theory I believe the Parade chip can run in a less "automatic" mode
where everything is configured and controlled by Linux. I'd really
have preferred if we could have gotten that done, but it didn't end up
happening. :(
-Doug
Again this is not to block this change (you already have my ack) but
perhaps to help you debug this for future reference as we wont know what
change can break parade even in the future if this happens again and the
compatible check for parade will keep growing.
One suggestion I can give is can we read any status bits out of the
parade chip in the power_on() method as its right after the new method
to check why its not in good status or what error bits it throws and
perhaps share those error bits with the FAE to see when this error can
come to decode this black box a bit :)
From those bits, we can narrow down why this timing or sequence change
is affecting them. Like some things could be somehow this is delaying
the video pixels from transmitting, it expects PHY to be in some state
etc and we can kind-of reverse engineer why this change broke it.
Like-wise, it is highly possible at the moment we have identified only
parade not to work but if there is something wrong with this change we
can atleast know what and address it.
Thanks
Abhinav
Re: [PATCH 2/2] drm/msm: push IRQ setup into individual drivers
On 5/6/2022 6:00 PM, Dmitry Baryshkov wrote:
Afther the commit f026e431cf86 ("drm/msm: Convert to Linux IRQ
interfaces") converted MSM DRM driver to handle IRQs on it's own (rather
than using the DRM IRQ mid-layer), there is little point in keeping
irq wrapper in the msm_drv.c which just call into individual drivers.
Push respective code into the mdp4/mdp5/dpu drivers and drop
irq_preinstall/irq_postinstall/irq msm_kms funcs.
Isnt this change causing a lot of code duplication across mdp5/dpu/mdp4?
Do you have any future plans with respect to this separation?
I am missing why this separation into respective mdp4/5/dpu is necessary
because struct msm_kms remains a common struct in all of this so it
remaining in msm_drv will avoid duplication.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h | 13 +---
.../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 28 -
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 7 +++
drivers/gpu/drm/msm/disp/mdp4/mdp4_irq.c | 30 -
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 4 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.h | 4 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_irq.c | 30 -
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 4 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 4 +-
drivers/gpu/drm/msm/msm_drv.c | 62 +++
drivers/gpu/drm/msm/msm_kms.h | 4 +-
12 files changed, 105 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
index b5b6e7031fb9..c6938b1f1870 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_irq.h
@@ -8,13 +8,6 @@
#include "dpu_kms.h"
#include "dpu_hw_interrupts.h"
-/**
- * dpu_core_irq_preinstall - perform pre-installation of core IRQ handler
- * @kms: MSM KMS handle
- * @return:none
- */
-void dpu_core_irq_preinstall(struct msm_kms *kms);
-
/**
* dpu_core_irq_uninstall - uninstall core IRQ handler
* @kms: MSM KMS handle
@@ -23,11 +16,11 @@ void dpu_core_irq_preinstall(struct msm_kms *kms);
void dpu_core_irq_uninstall(struct msm_kms *kms);
/**
- * dpu_core_irq - core IRQ handler
+ * dpu_core_irq_install - install core IRQ handler
* @kms: MSM KMS handle
- * @return:interrupt handling status
+ * @return:non-zero in case of an error
*/
-irqreturn_t dpu_core_irq(struct msm_kms *kms);
+int dpu_core_irq_install(struct msm_kms *kms);
/**
* dpu_core_irq_read - IRQ helper function for reading IRQ status
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
index d6498e45dc2c..fa4f99034a08 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
@@ -164,8 +164,9 @@ static void dpu_core_irq_callback_handler(struct dpu_kms
*dpu_kms, int irq_idx)
dpu_kms->hw_intr->irq_tbl[irq_idx].cb(dpu_kms->hw_intr->irq_tbl[irq_idx].arg,
irq_idx);
}
-irqreturn_t dpu_core_irq(struct msm_kms *kms)
+static irqreturn_t dpu_irq(int irq, void *arg)
{
+ struct msm_kms *kms = arg;
struct dpu_kms *dpu_kms = to_dpu_kms(kms);
struct dpu_hw_intr *intr = dpu_kms->hw_intr;
int reg_idx;
@@ -541,7 +542,7 @@ void dpu_debugfs_core_irq_init(struct dpu_kms *dpu_kms,
}
#endif
-void dpu_core_irq_preinstall(struct msm_kms *kms)
+static void dpu_core_irq_preinstall(struct msm_kms *kms)
{
struct dpu_kms *dpu_kms = to_dpu_kms(kms);
int i;
@@ -570,5 +571,28 @@ void dpu_core_irq_uninstall(struct msm_kms *kms)
dpu_clear_irqs(dpu_kms);
dpu_disable_all_irqs(dpu_kms);
+ if (kms->irq_requested)
+ free_irq(kms->irq, kms);
pm_runtime_put_sync(_kms->pdev->dev);
}
+
+int dpu_core_irq_install(struct msm_kms *kms)
+{
+ int ret;
+
+ dpu_core_irq_preinstall(kms);
+
+ ret = request_irq(kms->irq, dpu_irq, 0, "dpu", kms);
+ if (ret)
+ return ret;
+
+ kms->irq_requested = true;
+
+ ret = dpu_irq_postinstall(kms);
+ if (ret) {
+ free_irq(kms->irq, kms);
+ return ret;
+ }
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 2b9d931474e0..494978da7785 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -884,7 +884,7 @@ static void dpu_kms_destroy(struct msm_kms *kms)
pm_runtime_disable(_kms->pdev->dev);
}
-static int dpu_irq_postinstall(struct msm_kms *kms)
+int dpu_irq_postinstall(struct msm_kms *kms)
{
struct msm_drm_private *priv;
struct dpu_kms *dpu_kms = to_dpu_kms(kms);
@@ -960,10
Re: [PATCH v4] drm/msm/dsi: don't powerup at modeset time for parade-ps8640
Hi,
On Thu, May 12, 2022 at 3:34 PM Abhinav Kumar wrote:
>
> On 5/12/2022 3:16 PM, Dmitry Baryshkov wrote:
> > On 13/05/2022 01:00, Douglas Anderson wrote:
> >> Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
> >> time") caused sc7180 Chromebooks that use the parade-ps8640 bridge
> >> chip to fail to turn the display back on after it turns off.
> >>
> >> Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to
> >> handle the new power sequence. The Linux driver has almost nothing in
> >> it and most of the logic for this bridge chip is in black-box firmware
> >> that the bridge chip uses.
> >>
> >> Also unfortunately, reverting the patch will break "tc358762".
> >>
> >> The long term solution here is probably Dave Stevenson's series [1]
> >> that would give more flexibility. However, that is likely not a quick
> >> fix.
> >>
> >> For the short term, we'll look at the compatible of the next bridge in
> >> the chain and go back to the old way for the Parade PS8640 bridge
> >> chip. If it's found that other bridge chips also need this workaround
> >> then we can add them to the list or consider inverting the condition.
> >>
> >> [1]
> >> https://lore.kernel.org/r/cover.1646406653.git.dave.steven...@raspberrypi.com
> >>
> >>
> >> Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
> >> time")
> >> Suggested-by: Rob Clark
> >> Signed-off-by: Douglas Anderson
> >
> > Reviewed-by: Dmitry Baryshkov
> >
> Yes, I think this is a better solution than a full revert
>
> Reviewed-by: Abhinav Kumar
>
> I am curious to know why this doesnt work for parade but will not hold
> this patch back for that. We are initializing and turning on DSI PHY now
> before turning on the bridge chip which is actually better as we are
> putting PHY in a good state.
>
> So this should have been better, but somehow doesn't work.
I can't really explain it, but mostly because the Parade chip is just
a big black box. There have been several times when an OEM using this
bridge chip had one problem or another with getting the display to
turn on, then the parade FAE would make some magic tweak to the
firmware and it would be fixed. The current way that the Linux driver
is working is with pretty much zero configuration so I think this chip
bakes in a bunch of assumptions about the timings / signal coming from
the MIPI DSI side. It doesn't surprise me that changing the order like
this would confuse it.
In theory I believe the Parade chip can run in a less "automatic" mode
where everything is configured and controlled by Linux. I'd really
have preferred if we could have gotten that done, but it didn't end up
happening. :(
-Doug
Re: [PATCH v4] drm/msm/dsi: don't powerup at modeset time for parade-ps8640
On 5/12/2022 3:16 PM, Dmitry Baryshkov wrote:
On 13/05/2022 01:00, Douglas Anderson wrote:
Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
time") caused sc7180 Chromebooks that use the parade-ps8640 bridge
chip to fail to turn the display back on after it turns off.
Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to
handle the new power sequence. The Linux driver has almost nothing in
it and most of the logic for this bridge chip is in black-box firmware
that the bridge chip uses.
Also unfortunately, reverting the patch will break "tc358762".
The long term solution here is probably Dave Stevenson's series [1]
that would give more flexibility. However, that is likely not a quick
fix.
For the short term, we'll look at the compatible of the next bridge in
the chain and go back to the old way for the Parade PS8640 bridge
chip. If it's found that other bridge chips also need this workaround
then we can add them to the list or consider inverting the condition.
[1]
https://lore.kernel.org/r/cover.1646406653.git.dave.steven...@raspberrypi.com
Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
time")
Suggested-by: Rob Clark
Signed-off-by: Douglas Anderson
Reviewed-by: Dmitry Baryshkov
Yes, I think this is a better solution than a full revert
Reviewed-by: Abhinav Kumar
I am curious to know why this doesnt work for parade but will not hold
this patch back for that. We are initializing and turning on DSI PHY now
before turning on the bridge chip which is actually better as we are
putting PHY in a good state.
So this should have been better, but somehow doesnt work.
---
Note that, unlike `struct device`, `struct drm_bridge` still has a
`#ifdef` around the `of_node`. The extra stub function in this patch
is to make sure that we can pass COMPILE_TEST, not because I expect
that we'll actually run into real users who are running this driver
without device tree.
Changes in v4:
- Use the compatible string of the next bridge as per Rob.
Changes in v3:
- No longer a revert; now a module parameter.
Changes in v2:
- Remove the mud from my face.
drivers/gpu/drm/msm/dsi/dsi_manager.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 50b987658b1f..2cabba65a8f1 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -34,6 +34,26 @@ static struct msm_dsi_manager msm_dsim_glb;
#define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
#define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id
== id)
+#ifdef CONFIG_OF
+static bool dsi_mgr_power_on_early(struct drm_bridge *bridge)
+{
+ struct drm_bridge *next_bridge = drm_bridge_get_next_bridge(bridge);
+
+ /*
+ * If the next bridge in the chain is the Parade ps8640 bridge chip
+ * then don't power on early since it seems to violate the
expectations
+ * of the firmware that the bridge chip is running.
+ */
+ return !(next_bridge && next_bridge->of_node &&
+ of_device_is_compatible(next_bridge->of_node,
"parade,ps8640"));
+}
+#else
+static inline bool dsi_mgr_power_on_early(struct drm_bridge *bridge)
+{
+ return true;
+}
+#endif
+
static inline struct msm_dsi *dsi_mgr_get_dsi(int id)
{
return msm_dsim_glb.dsi[id];
@@ -389,6 +409,9 @@ static void dsi_mgr_bridge_pre_enable(struct
drm_bridge *bridge)
if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id))
return;
+ if (!dsi_mgr_power_on_early(bridge))
+ dsi_mgr_bridge_power_on(bridge);
+
/* Always call panel functions once, because even for dual panels,
* there is only one drm_panel instance.
*/
@@ -570,7 +593,8 @@ static void dsi_mgr_bridge_mode_set(struct
drm_bridge *bridge,
if (is_bonded_dsi && other_dsi)
msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode);
- dsi_mgr_bridge_power_on(bridge);
+ if (dsi_mgr_power_on_early(bridge))
+ dsi_mgr_bridge_power_on(bridge);
}
static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct
drm_bridge *bridge,
Re: [PATCH v1 13/15] mm: handling Non-LRU pages returned by vm_normal_pages
On 5/11/2022 1:50 PM, Jason Gunthorpe wrote: On Thu, May 05, 2022 at 04:34:36PM -0500, Alex Sierra wrote: diff --git a/mm/memory.c b/mm/memory.c index 76e3af9639d9..892c4cc54dc2 100644 +++ b/mm/memory.c @@ -621,6 +621,13 @@ struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr, if (is_zero_pfn(pfn)) return NULL; if (pte_devmap(pte)) +/* + * NOTE: Technically this should goto check_pfn label. However, page->_mapcount + * is never incremented for device pages that are mmap through DAX mechanism + * using pmem driver mounted into ext4 filesystem. When these pages are unmap, + * zap_pte_range is called and vm_normal_page return a valid page with + * page_mapcount() = 0, before page_remove_rmap is called. + */ return NULL; ? Where does this series cause device coherent to be returned? In our case, device coherent pages could be obtained as a result of migration(Patches 6/7 of 15), ending up mapped in CPU page tables. Later on, these pages might need to be returned by get_user_pages or other callers through vm_normal_pages. Our approach in this series, is to handle device-coherent-managed pages returned by vm_normal_pages, inside each caller. EX. device coherent pages don’t support LRU lists, NUMA migration or THP. Wasn't the plan to not set pte_devmap() ? amdgpu does not set pte_devmap for our DEVICE_COHERENT pages. DEVMAP flags are set by drivers like virtio_fs or pmem, where MEMORY_DEVICE_FS_DAX type is used. This patch series deals with DEVICE_COHERENT pages. My understanding was, that the DAX code and DEVICE_GENERIC would be fixed up later by someone more familiar with it. Were you expecting that we'd fix the DAX usage of pte_devmap flags in this patch series as well? Regards, Alex Sierra Jason
Re: [PATCH v4] drm/msm/dsi: don't powerup at modeset time for parade-ps8640
On 13/05/2022 01:00, Douglas Anderson wrote:
Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
time") caused sc7180 Chromebooks that use the parade-ps8640 bridge
chip to fail to turn the display back on after it turns off.
Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to
handle the new power sequence. The Linux driver has almost nothing in
it and most of the logic for this bridge chip is in black-box firmware
that the bridge chip uses.
Also unfortunately, reverting the patch will break "tc358762".
The long term solution here is probably Dave Stevenson's series [1]
that would give more flexibility. However, that is likely not a quick
fix.
For the short term, we'll look at the compatible of the next bridge in
the chain and go back to the old way for the Parade PS8640 bridge
chip. If it's found that other bridge chips also need this workaround
then we can add them to the list or consider inverting the condition.
[1]
https://lore.kernel.org/r/cover.1646406653.git.dave.steven...@raspberrypi.com
Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset time")
Suggested-by: Rob Clark
Signed-off-by: Douglas Anderson
Reviewed-by: Dmitry Baryshkov
---
Note that, unlike `struct device`, `struct drm_bridge` still has a
`#ifdef` around the `of_node`. The extra stub function in this patch
is to make sure that we can pass COMPILE_TEST, not because I expect
that we'll actually run into real users who are running this driver
without device tree.
Changes in v4:
- Use the compatible string of the next bridge as per Rob.
Changes in v3:
- No longer a revert; now a module parameter.
Changes in v2:
- Remove the mud from my face.
drivers/gpu/drm/msm/dsi/dsi_manager.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 50b987658b1f..2cabba65a8f1 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -34,6 +34,26 @@ static struct msm_dsi_manager msm_dsim_glb;
#define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
#define IS_MASTER_DSI_LINK(id)(msm_dsim_glb.master_dsi_link_id == id)
+#ifdef CONFIG_OF
+static bool dsi_mgr_power_on_early(struct drm_bridge *bridge)
+{
+ struct drm_bridge *next_bridge = drm_bridge_get_next_bridge(bridge);
+
+ /*
+* If the next bridge in the chain is the Parade ps8640 bridge chip
+* then don't power on early since it seems to violate the expectations
+* of the firmware that the bridge chip is running.
+*/
+ return !(next_bridge && next_bridge->of_node &&
+of_device_is_compatible(next_bridge->of_node,
"parade,ps8640"));
+}
+#else
+static inline bool dsi_mgr_power_on_early(struct drm_bridge *bridge)
+{
+ return true;
+}
+#endif
+
static inline struct msm_dsi *dsi_mgr_get_dsi(int id)
{
return msm_dsim_glb.dsi[id];
@@ -389,6 +409,9 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge
*bridge)
if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id))
return;
+ if (!dsi_mgr_power_on_early(bridge))
+ dsi_mgr_bridge_power_on(bridge);
+
/* Always call panel functions once, because even for dual panels,
* there is only one drm_panel instance.
*/
@@ -570,7 +593,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge
*bridge,
if (is_bonded_dsi && other_dsi)
msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode);
- dsi_mgr_bridge_power_on(bridge);
+ if (dsi_mgr_power_on_early(bridge))
+ dsi_mgr_bridge_power_on(bridge);
}
static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
--
With best wishes
Dmitry
Re: [PATCH v3] drm/msm/dsi: only powerup at modeset time if "early_poweron" modparam
Hi,
On Thu, May 12, 2022 at 1:59 PM Dmitry Baryshkov
wrote:
>
> On 12/05/2022 23:52, Douglas Anderson wrote:
> > Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
> > time") caused sc7180 Chromebooks that use the parade-ps8640 bridge
> > chip to fail to turn the display back on after it turns off.
> >
> > Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to
> > handle the new power sequence. The Linux driver has almost nothing in
> > it and most of the logic for this bridge chip is in black-box firmware
> > that the bridge chip uses.
> >
> > Also unfortunately, reverting the patch will break "tc358762".
> >
> > The long term solution here is probably Dave Stevenson's series [1]
> > that would give more flexibility. However, that is likely not a quick
> > fix.
> >
> > For the short term, let's introduce a module parameter that selects
> > between the two behaviors. This is a short term hack but at least can
> > keep both users working. We'll default the value of the module
> > parameter to the old behavior. Given that the old behavior has existed
> > for longer it's probably a safer default.
> >
> > [1]
> > https://lore.kernel.org/r/cover.1646406653.git.dave.steven...@raspberrypi.com
> >
> > Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset time")
> > Suggested-by: Dmitry Baryshkov
> > Signed-off-by: Douglas Anderson
>
> Reviewed-by: Dmitry Baryshkov
>
> Two minor issues below.
I only saw one, so hopefully I didn't miss another request. ;-)
> > ---
> >
> > Changes in v3:
> > - No longer a revert; now a module parameter.
> >
> > Changes in v2:
> > - Remove the mud from my face.
> >
> > drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 +-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > index 50b987658b1f..2bf4123ef5df 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > @@ -34,6 +34,10 @@ static struct msm_dsi_manager msm_dsim_glb;
> > #define IS_SYNC_NEEDED()(msm_dsim_glb.is_sync_needed)
> > #define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id ==
> > id)
> >
> > +bool early_poweron;
> > +MODULE_PARM_DESC(early_poweron, "Power DSI controller early");
> > +module_param(early_poweron, bool, 0600);
>
> Nit: dsi_early_poweron (to be clear that it related to DSI only).
>
> I thought about suggesting 'dsi_no_early_poweron' instead to catch
> possible issues with other bridges. But... I think with Dave's series
> will have to enable bridges one by one, so it doesn't make real sense.
I made the change you requested and was about to send out a v4 when
offline Rob pointed out a better way that seems to work. I can use the
`compatible` of the next bridge. I've sent out v4 as per Rob's
suggestion:
https://lore.kernel.org/r/20220512145954.v4.1.Ia196e35ad985059e77b038a41662faae9e26f411@changeid
-Doug
[PATCH v4] drm/msm/dsi: don't powerup at modeset time for parade-ps8640
Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
time") caused sc7180 Chromebooks that use the parade-ps8640 bridge
chip to fail to turn the display back on after it turns off.
Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to
handle the new power sequence. The Linux driver has almost nothing in
it and most of the logic for this bridge chip is in black-box firmware
that the bridge chip uses.
Also unfortunately, reverting the patch will break "tc358762".
The long term solution here is probably Dave Stevenson's series [1]
that would give more flexibility. However, that is likely not a quick
fix.
For the short term, we'll look at the compatible of the next bridge in
the chain and go back to the old way for the Parade PS8640 bridge
chip. If it's found that other bridge chips also need this workaround
then we can add them to the list or consider inverting the condition.
[1]
https://lore.kernel.org/r/cover.1646406653.git.dave.steven...@raspberrypi.com
Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset time")
Suggested-by: Rob Clark
Signed-off-by: Douglas Anderson
---
Note that, unlike `struct device`, `struct drm_bridge` still has a
`#ifdef` around the `of_node`. The extra stub function in this patch
is to make sure that we can pass COMPILE_TEST, not because I expect
that we'll actually run into real users who are running this driver
without device tree.
Changes in v4:
- Use the compatible string of the next bridge as per Rob.
Changes in v3:
- No longer a revert; now a module parameter.
Changes in v2:
- Remove the mud from my face.
drivers/gpu/drm/msm/dsi/dsi_manager.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 50b987658b1f..2cabba65a8f1 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -34,6 +34,26 @@ static struct msm_dsi_manager msm_dsim_glb;
#define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
#define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
+#ifdef CONFIG_OF
+static bool dsi_mgr_power_on_early(struct drm_bridge *bridge)
+{
+ struct drm_bridge *next_bridge = drm_bridge_get_next_bridge(bridge);
+
+ /*
+* If the next bridge in the chain is the Parade ps8640 bridge chip
+* then don't power on early since it seems to violate the expectations
+* of the firmware that the bridge chip is running.
+*/
+ return !(next_bridge && next_bridge->of_node &&
+of_device_is_compatible(next_bridge->of_node,
"parade,ps8640"));
+}
+#else
+static inline bool dsi_mgr_power_on_early(struct drm_bridge *bridge)
+{
+ return true;
+}
+#endif
+
static inline struct msm_dsi *dsi_mgr_get_dsi(int id)
{
return msm_dsim_glb.dsi[id];
@@ -389,6 +409,9 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge
*bridge)
if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id))
return;
+ if (!dsi_mgr_power_on_early(bridge))
+ dsi_mgr_bridge_power_on(bridge);
+
/* Always call panel functions once, because even for dual panels,
* there is only one drm_panel instance.
*/
@@ -570,7 +593,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge
*bridge,
if (is_bonded_dsi && other_dsi)
msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode);
- dsi_mgr_bridge_power_on(bridge);
+ if (dsi_mgr_power_on_early(bridge))
+ dsi_mgr_bridge_power_on(bridge);
}
static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge
*bridge,
--
2.36.0.550.gb090851708-goog
Re: [PATCH 3/3] drm/panel: introduce ebbg,ft8719 panel
On Fri, May 6, 2022 at 2:18 PM Joel Selvaraj wrote:
> Add DRM panel driver for EBBG FT8719 6.18" 2246x1080 DSI video mode
> panel, which can be found on some Xiaomi Poco F1 phones. The panel's
> backlight is managed through QCOM WLED driver.
>
> Signed-off-by: Joel Selvaraj
Cool!
> +#define dsi_generic_write_seq(dsi, seq...) do {
> \
> + static const u8 d[] = { seq }; \
> + int ret;\
> + ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));\
> + if (ret < 0)\
> + return ret; \
> + } while (0)
> +
> +#define dsi_dcs_write_seq(dsi, seq...) do {\
> + static const u8 d[] = { seq }; \
> + int ret;\
> + ret = mipi_dsi_dcs_write_buffer(dsi, d, ARRAY_SIZE(d)); \
> + if (ret < 0)\
> + return ret; \
> + } while (0)
First I don't see what the do {} while (0) buys you, just use
a basic block {}.
Second look at mipi_dbi_command() in include/drm/drm_mipi_dbi.h
this is very similar.
So this utility macro should be in a generic file such as
include/drm/drm_mipi_dsi.h. (Can be added in a separate
patch.)
Third I think you need only one macro (see below).
> +static int ebbg_ft8719_on(struct ebbg_ft8719 *ctx)
> +{
> + struct mipi_dsi_device *dsi = ctx->dsi;
> + struct device *dev = >dev;
> + int ret;
> +
> + dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> + dsi_dcs_write_seq(dsi, 0x00, 0x00);
> + dsi_generic_write_seq(dsi, 0xff, 0x87, 0x19, 0x01);
It's dubious that you always have dsi_dcs_write_seq()
followed by dsi_generic_write_seq().
That means mipi_dsi_generic_write() followed by
mipi_dsi_dcs_write_buffer(). But if you look at these
commands in drivers/gpu/drm/drm_mipi_dsi.c
you see that they do the same thing!
Doesn't it work to combine them into one call for each
pair?
> + dsi_dcs_write_seq(dsi, 0x00, 0x80);
> + dsi_generic_write_seq(dsi, 0xff, 0x87, 0x19);
Lots of magic numbers. You don't have a datasheet do you?
So you could #define some of the magic?
> + if (ctx->prepared)
> + return 0;
(...)
> + ctx->prepared = true;
> + return 0;
(...)
> + if (!ctx->prepared)
> + return 0;
(...)
> + ctx->prepared = false;
> + return 0;
Drop this state variable it is a reimplementation of something
that the core will track for you.
The rest looks nice!
Yours,
Linus Walleij
Re: [PATCH] drm/amdgpu: Move mutex_init(>message_lock) to smu_early_init()
Applied. Thanks! Alex On Thu, May 12, 2022 at 4:45 PM Hans de Goede wrote: > > Lockdep complains about the smu->message_lock mutex being used before > it is initialized through the following call path: > > amdgpu_device_init() > amdgpu_dpm_mode2_reset() > smu_mode2_reset() >smu_v12_0_mode2_reset() > smu_cmn_send_smc_msg_with_param() > > Move the mutex_init() call to smu_early_init() to fix the mutex being > used before it is initialized. > > This fixes the following lockdep splat: > > [3.867331] [ cut here ] > [3.867335] fbcon: Taking over console > [3.867338] DEBUG_LOCKS_WARN_ON(lock->magic != lock) > [3.867340] WARNING: CPU: 14 PID: 491 at kernel/locking/mutex.c:579 > __mutex_lock+0x44c/0x830 > [3.867349] Modules linked in: amdgpu(+) crct10dif_pclmul drm_ttm_helper > crc32_pclmul ttm crc32c_intel ghash_clmulni_intel hid_lg_g15 iommu_v2 > sp5100_tco nvme gpu_sched drm_dp_helper nvme_core ccp wmi video > hid_logitech_dj ip6_tables ip_tables ipmi_devintf ipmi_msghandler fuse i2c_dev > [3.867363] CPU: 14 PID: 491 Comm: systemd-udevd Tainted: G I > 5.18.0-rc5+ #33 > [3.867366] Hardware name: Micro-Star International Co., Ltd. > MS-7C95/B550M PRO-VDH WIFI (MS-7C95), BIOS 2.90 12/23/2021 > [3.867369] RIP: 0010:__mutex_lock+0x44c/0x830 > [3.867372] Code: ff 85 c0 0f 84 33 fc ff ff 8b 0d b7 50 25 01 85 c9 0f 85 > 25 fc ff ff 48 c7 c6 fb 41 82 99 48 c7 c7 6b 63 80 99 e8 88 2a f8 ff <0f> 0b > e9 0b fc ff ff f6 83 b9 0c 00 00 01 0f 85 64 ff ff ff 4c 89 > [3.867377] RSP: 0018:aef8c0fc79f0 EFLAGS: 00010286 > [3.867380] RAX: 0028 RBX: RCX: > 0027 > [3.867382] RDX: 9ccc0dda0928 RSI: 0001 RDI: > 9ccc0dda0920 > [3.867384] RBP: aef8c0fc7a80 R08: R09: > aef8c0fc7820 > [3.867386] R10: 0003 R11: 9ccc2a2fffe8 R12: > 0002 > [3.867388] R13: 9cc990808058 R14: R15: > 9cc98bfc > [3.867390] FS: 7fc4d830f580() GS:9ccc0dd8() > knlGS: > [3.867394] CS: 0010 DS: ES: CR0: 80050033 > [3.867396] CR2: 560a77031410 CR3: 00010f522000 CR4: > 00750ee0 > [3.867398] PKRU: 5554 > [3.867399] Call Trace: > [3.867401] > [3.867403] ? smu_cmn_send_smc_msg_with_param+0x98/0x240 [amdgpu] > [3.867533] ? __mutex_lock+0x90/0x830 > [3.867535] ? amdgpu_dpm_mode2_reset+0x37/0x60 [amdgpu] > [3.867653] ? smu_cmn_send_smc_msg_with_param+0x98/0x240 [amdgpu] > [3.867758] smu_cmn_send_smc_msg_with_param+0x98/0x240 [amdgpu] > [3.867857] smu_mode2_reset+0x2b/0x50 [amdgpu] > [3.867953] amdgpu_dpm_mode2_reset+0x46/0x60 [amdgpu] > [3.868096] amdgpu_device_init.cold+0x1069/0x1e78 [amdgpu] > [3.868219] ? _raw_spin_unlock_irqrestore+0x30/0x50 > [3.868222] ? pci_conf1_read+0x9b/0xf0 > [3.868226] amdgpu_driver_load_kms+0x15/0x110 [amdgpu] > [3.868314] amdgpu_pci_probe+0x1a9/0x3c0 [amdgpu] > [3.868398] local_pci_probe+0x41/0x80 > [3.868401] pci_device_probe+0xab/0x200 > [3.868404] really_probe+0x1a1/0x370 > [3.868407] __driver_probe_device+0xfc/0x170 > [3.868410] driver_probe_device+0x1f/0x90 > [3.868412] __driver_attach+0xbf/0x1a0 > [3.868414] ? __device_attach_driver+0xe0/0xe0 > [3.868416] bus_for_each_dev+0x65/0x90 > [3.868419] bus_add_driver+0x151/0x1f0 > [3.868421] driver_register+0x89/0xd0 > [3.868423] ? 0xc0bd4000 > [3.868425] do_one_initcall+0x5d/0x300 > [3.868428] ? do_init_module+0x22/0x240 > [3.868431] ? rcu_read_lock_sched_held+0x3c/0x70 > [3.868434] ? trace_kmalloc+0x30/0xe0 > [3.868437] ? kmem_cache_alloc_trace+0x1e6/0x3a0 > [3.868440] do_init_module+0x4a/0x240 > [3.868442] __do_sys_finit_module+0x93/0xf0 > [3.868446] do_syscall_64+0x5b/0x80 > [3.868449] ? rcu_read_lock_sched_held+0x3c/0x70 > [3.868451] ? lockdep_hardirqs_on_prepare+0xd9/0x180 > [3.868454] ? do_syscall_64+0x67/0x80 > [3.868456] ? do_syscall_64+0x67/0x80 > [3.868458] ? do_syscall_64+0x67/0x80 > [3.868460] ? do_syscall_64+0x67/0x80 > [3.868462] entry_SYSCALL_64_after_hwframe+0x44/0xae > [3.868465] RIP: 0033:0x7fc4d8ec1ced > [3.868467] Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 > f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d > 01 f0 ff ff 73 01 c3 48 8b 0d fb 70 0e 00 f7 d8 64 89 01 48 > [3.868472] RSP: 002b:7fff687ae6b8 EFLAGS: 0246 ORIG_RAX: > 0139 > [3.868475] RAX: ffda RBX: 560a76fbca60 RCX: > 7fc4d8ec1ced > [3.868477] RDX: RSI: 7fc4d902343c RDI: > 0011 > [3.868479] RBP: 7fc4d902343c R08: R09: > 560a76fb59c0 > [3.868481] R10: 0011 R11:
Re: [PATCH] drm/i915: Fix CFI violation with show_dynamic_id()
On May 12, 2022 2:17:04 PM PDT, Nathan Chancellor wrote:
>When an attribute group is created with sysfs_create_group(), the
>->sysfs_ops() callback is set to kobj_sysfs_ops, which sets the ->show()
>callback to kobj_attr_show(). kobj_attr_show() uses container_of() to
>get the ->show() callback from the attribute it was passed, meaning the
>->show() callback needs to be the same type as the ->show() callback in
>'struct kobj_attribute'.
>
>However, show_dynamic_id() has the type of the ->show() callback in
>'struct device_attribute', which causes a CFI violation when opening the
>'id' sysfs node under drm/card0/metrics. This happens to work because
>the layout of 'struct kobj_attribute' and 'struct device_attribute' are
>the same, so the container_of() cast happens to allow the ->show()
>callback to still work.
>
>Change the type of show_dynamic_id() to match the ->show() callback in
>'struct kobj_attributes' and update the type of sysfs_metric_id to
>match, which resolves the CFI violation.
>
>Fixes: f89823c21224 ("drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG
>interface")
>Signed-off-by: Nathan Chancellor
This matches my own investigation into the error. Thanks for putting the patch
together! :)
Reviewed-by: Kees Cook
--
Kees Cook
[PATCH] drm/i915: Fix CFI violation with show_dynamic_id()
When an attribute group is created with sysfs_create_group(), the
->sysfs_ops() callback is set to kobj_sysfs_ops, which sets the ->show()
callback to kobj_attr_show(). kobj_attr_show() uses container_of() to
get the ->show() callback from the attribute it was passed, meaning the
->show() callback needs to be the same type as the ->show() callback in
'struct kobj_attribute'.
However, show_dynamic_id() has the type of the ->show() callback in
'struct device_attribute', which causes a CFI violation when opening the
'id' sysfs node under drm/card0/metrics. This happens to work because
the layout of 'struct kobj_attribute' and 'struct device_attribute' are
the same, so the container_of() cast happens to allow the ->show()
callback to still work.
Change the type of show_dynamic_id() to match the ->show() callback in
'struct kobj_attributes' and update the type of sysfs_metric_id to
match, which resolves the CFI violation.
Fixes: f89823c21224 ("drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG
interface")
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/i915_perf.c | 4 ++--
drivers/gpu/drm/i915/i915_perf_types.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 0a9c3fcc09b1..1577ab6754db 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -4050,8 +4050,8 @@ static struct i915_oa_reg *alloc_oa_regs(struct i915_perf
*perf,
return ERR_PTR(err);
}
-static ssize_t show_dynamic_id(struct device *dev,
- struct device_attribute *attr,
+static ssize_t show_dynamic_id(struct kobject *kobj,
+ struct kobj_attribute *attr,
char *buf)
{
struct i915_oa_config *oa_config =
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h
b/drivers/gpu/drm/i915/i915_perf_types.h
index 473a3c0544bb..05cb9a335a97 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -55,7 +55,7 @@ struct i915_oa_config {
struct attribute_group sysfs_metric;
struct attribute *attrs[2];
- struct device_attribute sysfs_metric_id;
+ struct kobj_attribute sysfs_metric_id;
struct kref ref;
struct rcu_head rcu;
base-commit: 7ecc3cc8a7b39f08eee9aea7b718187583342a70
--
2.36.1
Re: [PATCH v3] drm/msm/dsi: only powerup at modeset time if "early_poweron" modparam
On 12/05/2022 23:52, Douglas Anderson wrote:
Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
time") caused sc7180 Chromebooks that use the parade-ps8640 bridge
chip to fail to turn the display back on after it turns off.
Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to
handle the new power sequence. The Linux driver has almost nothing in
it and most of the logic for this bridge chip is in black-box firmware
that the bridge chip uses.
Also unfortunately, reverting the patch will break "tc358762".
The long term solution here is probably Dave Stevenson's series [1]
that would give more flexibility. However, that is likely not a quick
fix.
For the short term, let's introduce a module parameter that selects
between the two behaviors. This is a short term hack but at least can
keep both users working. We'll default the value of the module
parameter to the old behavior. Given that the old behavior has existed
for longer it's probably a safer default.
[1]
https://lore.kernel.org/r/cover.1646406653.git.dave.steven...@raspberrypi.com
Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset time")
Suggested-by: Dmitry Baryshkov
Signed-off-by: Douglas Anderson
Reviewed-by: Dmitry Baryshkov
Two minor issues below.
---
Changes in v3:
- No longer a revert; now a module parameter.
Changes in v2:
- Remove the mud from my face.
drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 50b987658b1f..2bf4123ef5df 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -34,6 +34,10 @@ static struct msm_dsi_manager msm_dsim_glb;
#define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
#define IS_MASTER_DSI_LINK(id)(msm_dsim_glb.master_dsi_link_id == id)
+bool early_poweron;
+MODULE_PARM_DESC(early_poweron, "Power DSI controller early");
+module_param(early_poweron, bool, 0600);
Nit: dsi_early_poweron (to be clear that it related to DSI only).
I thought about suggesting 'dsi_no_early_poweron' instead to catch
possible issues with other bridges. But... I think with Dave's series
will have to enable bridges one by one, so it doesn't make real sense.
+
static inline struct msm_dsi *dsi_mgr_get_dsi(int id)
{
return msm_dsim_glb.dsi[id];
@@ -389,6 +393,9 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge
*bridge)
if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id))
return;
+ if (!early_poweron)
+ dsi_mgr_bridge_power_on(bridge);
+
/* Always call panel functions once, because even for dual panels,
* there is only one drm_panel instance.
*/
@@ -570,7 +577,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge
*bridge,
if (is_bonded_dsi && other_dsi)
msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode);
- dsi_mgr_bridge_power_on(bridge);
+ if (early_poweron)
+ dsi_mgr_bridge_power_on(bridge);
}
static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge *bridge,
--
With best wishes
Dmitry
[PATCH v3] drm/msm/dsi: only powerup at modeset time if "early_poweron" modparam
Commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
time") caused sc7180 Chromebooks that use the parade-ps8640 bridge
chip to fail to turn the display back on after it turns off.
Unfortunately, it doesn't look easy to fix the parade-ps8640 driver to
handle the new power sequence. The Linux driver has almost nothing in
it and most of the logic for this bridge chip is in black-box firmware
that the bridge chip uses.
Also unfortunately, reverting the patch will break "tc358762".
The long term solution here is probably Dave Stevenson's series [1]
that would give more flexibility. However, that is likely not a quick
fix.
For the short term, let's introduce a module parameter that selects
between the two behaviors. This is a short term hack but at least can
keep both users working. We'll default the value of the module
parameter to the old behavior. Given that the old behavior has existed
for longer it's probably a safer default.
[1]
https://lore.kernel.org/r/cover.1646406653.git.dave.steven...@raspberrypi.com
Fixes: 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset time")
Suggested-by: Dmitry Baryshkov
Signed-off-by: Douglas Anderson
---
Changes in v3:
- No longer a revert; now a module parameter.
Changes in v2:
- Remove the mud from my face.
drivers/gpu/drm/msm/dsi/dsi_manager.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 50b987658b1f..2bf4123ef5df 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -34,6 +34,10 @@ static struct msm_dsi_manager msm_dsim_glb;
#define IS_SYNC_NEEDED() (msm_dsim_glb.is_sync_needed)
#define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
+bool early_poweron;
+MODULE_PARM_DESC(early_poweron, "Power DSI controller early");
+module_param(early_poweron, bool, 0600);
+
static inline struct msm_dsi *dsi_mgr_get_dsi(int id)
{
return msm_dsim_glb.dsi[id];
@@ -389,6 +393,9 @@ static void dsi_mgr_bridge_pre_enable(struct drm_bridge
*bridge)
if (is_bonded_dsi && !IS_MASTER_DSI_LINK(id))
return;
+ if (!early_poweron)
+ dsi_mgr_bridge_power_on(bridge);
+
/* Always call panel functions once, because even for dual panels,
* there is only one drm_panel instance.
*/
@@ -570,7 +577,8 @@ static void dsi_mgr_bridge_mode_set(struct drm_bridge
*bridge,
if (is_bonded_dsi && other_dsi)
msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode);
- dsi_mgr_bridge_power_on(bridge);
+ if (early_poweron)
+ dsi_mgr_bridge_power_on(bridge);
}
static enum drm_mode_status dsi_mgr_bridge_mode_valid(struct drm_bridge
*bridge,
--
2.36.0.550.gb090851708-goog
[PATCH] drm/amdgpu: Move mutex_init(>message_lock) to smu_early_init()
Lockdep complains about the smu->message_lock mutex being used before it is initialized through the following call path: amdgpu_device_init() amdgpu_dpm_mode2_reset() smu_mode2_reset() smu_v12_0_mode2_reset() smu_cmn_send_smc_msg_with_param() Move the mutex_init() call to smu_early_init() to fix the mutex being used before it is initialized. This fixes the following lockdep splat: [3.867331] [ cut here ] [3.867335] fbcon: Taking over console [3.867338] DEBUG_LOCKS_WARN_ON(lock->magic != lock) [3.867340] WARNING: CPU: 14 PID: 491 at kernel/locking/mutex.c:579 __mutex_lock+0x44c/0x830 [3.867349] Modules linked in: amdgpu(+) crct10dif_pclmul drm_ttm_helper crc32_pclmul ttm crc32c_intel ghash_clmulni_intel hid_lg_g15 iommu_v2 sp5100_tco nvme gpu_sched drm_dp_helper nvme_core ccp wmi video hid_logitech_dj ip6_tables ip_tables ipmi_devintf ipmi_msghandler fuse i2c_dev [3.867363] CPU: 14 PID: 491 Comm: systemd-udevd Tainted: G I 5.18.0-rc5+ #33 [3.867366] Hardware name: Micro-Star International Co., Ltd. MS-7C95/B550M PRO-VDH WIFI (MS-7C95), BIOS 2.90 12/23/2021 [3.867369] RIP: 0010:__mutex_lock+0x44c/0x830 [3.867372] Code: ff 85 c0 0f 84 33 fc ff ff 8b 0d b7 50 25 01 85 c9 0f 85 25 fc ff ff 48 c7 c6 fb 41 82 99 48 c7 c7 6b 63 80 99 e8 88 2a f8 ff <0f> 0b e9 0b fc ff ff f6 83 b9 0c 00 00 01 0f 85 64 ff ff ff 4c 89 [3.867377] RSP: 0018:aef8c0fc79f0 EFLAGS: 00010286 [3.867380] RAX: 0028 RBX: RCX: 0027 [3.867382] RDX: 9ccc0dda0928 RSI: 0001 RDI: 9ccc0dda0920 [3.867384] RBP: aef8c0fc7a80 R08: R09: aef8c0fc7820 [3.867386] R10: 0003 R11: 9ccc2a2fffe8 R12: 0002 [3.867388] R13: 9cc990808058 R14: R15: 9cc98bfc [3.867390] FS: 7fc4d830f580() GS:9ccc0dd8() knlGS: [3.867394] CS: 0010 DS: ES: CR0: 80050033 [3.867396] CR2: 560a77031410 CR3: 00010f522000 CR4: 00750ee0 [3.867398] PKRU: 5554 [3.867399] Call Trace: [3.867401] [3.867403] ? smu_cmn_send_smc_msg_with_param+0x98/0x240 [amdgpu] [3.867533] ? __mutex_lock+0x90/0x830 [3.867535] ? amdgpu_dpm_mode2_reset+0x37/0x60 [amdgpu] [3.867653] ? smu_cmn_send_smc_msg_with_param+0x98/0x240 [amdgpu] [3.867758] smu_cmn_send_smc_msg_with_param+0x98/0x240 [amdgpu] [3.867857] smu_mode2_reset+0x2b/0x50 [amdgpu] [3.867953] amdgpu_dpm_mode2_reset+0x46/0x60 [amdgpu] [3.868096] amdgpu_device_init.cold+0x1069/0x1e78 [amdgpu] [3.868219] ? _raw_spin_unlock_irqrestore+0x30/0x50 [3.868222] ? pci_conf1_read+0x9b/0xf0 [3.868226] amdgpu_driver_load_kms+0x15/0x110 [amdgpu] [3.868314] amdgpu_pci_probe+0x1a9/0x3c0 [amdgpu] [3.868398] local_pci_probe+0x41/0x80 [3.868401] pci_device_probe+0xab/0x200 [3.868404] really_probe+0x1a1/0x370 [3.868407] __driver_probe_device+0xfc/0x170 [3.868410] driver_probe_device+0x1f/0x90 [3.868412] __driver_attach+0xbf/0x1a0 [3.868414] ? __device_attach_driver+0xe0/0xe0 [3.868416] bus_for_each_dev+0x65/0x90 [3.868419] bus_add_driver+0x151/0x1f0 [3.868421] driver_register+0x89/0xd0 [3.868423] ? 0xc0bd4000 [3.868425] do_one_initcall+0x5d/0x300 [3.868428] ? do_init_module+0x22/0x240 [3.868431] ? rcu_read_lock_sched_held+0x3c/0x70 [3.868434] ? trace_kmalloc+0x30/0xe0 [3.868437] ? kmem_cache_alloc_trace+0x1e6/0x3a0 [3.868440] do_init_module+0x4a/0x240 [3.868442] __do_sys_finit_module+0x93/0xf0 [3.868446] do_syscall_64+0x5b/0x80 [3.868449] ? rcu_read_lock_sched_held+0x3c/0x70 [3.868451] ? lockdep_hardirqs_on_prepare+0xd9/0x180 [3.868454] ? do_syscall_64+0x67/0x80 [3.868456] ? do_syscall_64+0x67/0x80 [3.868458] ? do_syscall_64+0x67/0x80 [3.868460] ? do_syscall_64+0x67/0x80 [3.868462] entry_SYSCALL_64_after_hwframe+0x44/0xae [3.868465] RIP: 0033:0x7fc4d8ec1ced [3.868467] Code: 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d fb 70 0e 00 f7 d8 64 89 01 48 [3.868472] RSP: 002b:7fff687ae6b8 EFLAGS: 0246 ORIG_RAX: 0139 [3.868475] RAX: ffda RBX: 560a76fbca60 RCX: 7fc4d8ec1ced [3.868477] RDX: RSI: 7fc4d902343c RDI: 0011 [3.868479] RBP: 7fc4d902343c R08: R09: 560a76fb59c0 [3.868481] R10: 0011 R11: 0246 R12: 0002 [3.868484] R13: 560a76f8bfd0 R14: R15: 560a76fc2d10 [3.868487] [3.868489] irq event stamp: 120617 [3.868490] hardirqs last enabled at (120617): [] __up_console_sem+0x5e/0x70 [3.868494] hardirqs last
[PATCH v6] drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq_ctrl()
dp_catalog_ctrl_reset() will software reset DP controller. But it will
not reset programmable registers to default value. DP driver still have
to clear mask bits to interrupt status registers to disable interrupts
after software reset of controller.
At current implementation, dp_ctrl_reset_irq_ctrl() will software reset dp
controller but did not call dp_catalog_ctrl_enable_irq(false) to clear hpd
related interrupt mask bits to disable hpd related interrupts due to it
mistakenly think hpd related interrupt mask bits will be cleared by software
reset of dp controller automatically. This mistake may cause system to crash
during suspending procedure due to unexpected irq fired and trigger event
thread to access dp controller registers with controller clocks are disabled.
This patch fixes system crash during suspending problem by removing "enable"
flag condition checking at dp_ctrl_reset_irq_ctrl() so that hpd related
interrupt mask bits are cleared to prevent unexpected from happening.
In addition, this patch also add suspended flag to prevent new events be
added into event Q to wake up event thread after system suspended.
Changes in v2:
-- add more details commit text
Changes in v3:
-- add synchrons_irq()
-- add atomic_t suspended
Changes in v4:
-- correct Fixes's commit ID
-- remove synchrons_irq()
Changes in v5:
-- revise commit text
Changes in v6:
-- add event_lock to protect "suspended"
Fixes: 989ebe7bc446 ("drm/msm/dp: do not initialize phy until plugin interrupt
received")
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_ctrl.c| 9 +++--
drivers/gpu/drm/msm/dp/dp_display.c | 25 -
2 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index af7a80c..f3e333e 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1389,8 +1389,13 @@ void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl,
bool enable)
dp_catalog_ctrl_reset(ctrl->catalog);
- if (enable)
- dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
+ /*
+* all dp controller programmable registers will not
+* be reset to default value after DP_SW_RESET
+* therefore interrupt mask bits have to be updated
+* to enable/disable interrupts
+*/
+ dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
}
void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index c388323..ab691aa 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -98,6 +98,8 @@ struct dp_display_private {
struct dp_ctrl*ctrl;
struct dp_debug *debug;
+ bool suspended;
+
struct dp_usbpd_cb usbpd_cb;
struct dp_display_mode dp_mode;
struct msm_dp dp_display;
@@ -187,6 +189,11 @@ static int dp_add_event(struct dp_display_private
*dp_priv, u32 event,
int pndx;
spin_lock_irqsave(_priv->event_lock, flag);
+ if (dp_priv->suspended) {
+ spin_unlock_irqrestore(_priv->event_lock, flag);
+ return -ENOENT;
+ }
+
pndx = dp_priv->event_pndx + 1;
pndx %= DP_EVENT_Q_MAX;
if (pndx == dp_priv->event_gndx) {
@@ -454,7 +461,6 @@ static void dp_display_host_deinit(struct
dp_display_private *dp)
dp->dp_display.connector_type, dp->core_initialized,
dp->phy_initialized);
- dp_ctrl_reset_irq_ctrl(dp->ctrl, false);
dp_aux_deinit(dp->aux);
dp_power_deinit(dp->power);
dp->core_initialized = false;
@@ -1112,7 +1118,12 @@ static int hpd_event_thread(void *data)
wait_event_interruptible(dp_priv->event_q,
(dp_priv->event_pndx != dp_priv->event_gndx));
}
+
spin_lock_irqsave(_priv->event_lock, flag);
+ if (dp_priv->suspended) {
+ spin_unlock_irqrestore(_priv->event_lock, flag);
+ continue;
+ }
todo = _priv->event_list[dp_priv->event_gndx];
if (todo->delay) {
struct dp_event *todo_next;
@@ -1351,6 +1362,7 @@ static int dp_pm_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct msm_dp *dp_display = platform_get_drvdata(pdev);
struct dp_display_private *dp;
+ unsigned long flag;
int sink_count = 0;
dp = container_of(dp_display, struct dp_display_private, dp_display);
@@ -1362,6 +1374,10 @@ static int dp_pm_resume(struct device *dev)
dp->dp_display.connector_type, dp->core_initialized,
dp->phy_initialized, dp_display->power_on);
+ spin_lock_irqsave(>event_lock, flag);
+ dp->suspended = false;
+
Re: [PATCH v12, 00/17] media: mtk-vcodec: support for M8192 decoder
On Thu, May 12, 2022 at 10:19:33AM +0800, Yunfei Dong wrote: > This series adds support for mt8192 h264/vp8/vp9 decoder drivers. Firstly, > refactor > power/clock/interrupt interfaces for mt8192 is lat and core architecture. > > Secondly, add new functions to get frame buffer size and resolution according > to decoder capability from scp side. Then add callback function to get/put > capture buffer in order to enable lat and core decoder in parallel, need to > adjust GStreamer at the same time. > > Then add to support MT21C compressed mode and fix v4l2-compliance fail. > > Next, extract H264 request api driver to let mt8183 and mt8192 use the same > code, and adds mt8192 frame based h264 driver for stateless decoder. > > Lastly, add vp8 and vp9 stateless decoder drivers. > > Patches 1 refactor power/clock/interrupt interface. > Patches 2~4 get frame buffer size and resolution according to decoder > capability. > Patches 5 set capture queue bytesused. > Patches 6 adjust GStreamer. > Patch 7~11 add to support MT21C compressed mode and fix v4l2-compliance fail. > patch 12 record capture queue format type. > Patch 13~14 extract h264 driver and add mt8192 frame based driver for h264 > decoder. > Patch 15~16 add vp8 and vp9 stateless decoder drivers. > Patch 17 prevent kernel crash when rmmod mtk-vcodec-dec.ko Hi Yunfei, With this series, and the new scp.img for mt8192 [1] (still waiting to get merged), I was able to get the following fluster scores on mt8192-asurada-spherion: VP8: 59/61 VP9: 249/303 H.264: 92/135 So for the whole series: Tested-by: Nícolas F. R. A. Prado Thanks, Nícolas [1] https://lore.kernel.org/all/2537b84fbba82a77ee0a517b12bdcdd5e6ac1503.ca...@mediatek.com/
Re: [PATCH v5] drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq_ctrl()
On 5/11/2022 6:03 PM, Dmitry Baryshkov wrote:
On Thu, 12 May 2022 at 04:01, Stephen Boyd wrote:
Quoting Dmitry Baryshkov (2022-05-11 17:41:50)
On 12/05/2022 03:02, Kuogee Hsieh wrote:
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index af7a80c..f3e333e 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1389,8 +1389,13 @@ void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl,
bool enable)
dp_catalog_ctrl_reset(ctrl->catalog);
- if (enable)
- dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
+ /*
+ * all dp controller programmable registers will not
+ * be reset to default value after DP_SW_RESET
+ * therefore interrupt mask bits have to be updated
+ * to enable/disable interrupts
+ */
+ dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
}
void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index c388323..79439b8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -98,6 +98,8 @@ struct dp_display_private {
struct dp_ctrl*ctrl;
struct dp_debug *debug;
+ atomic_t suspended;
I think it'd be better to protect it with event_lock rather than using
atomics.
Agreed. I think the concern is that the event queue will have "stuff" in
it. If the event queue was all a threaded irq we could simply call
synchronize_irq() after disabling the irq bit in the DP hardware and
then we would know it is safe to power down the DP logic. Unfortunately
the event queue is a kthread so we can't do that and we have to rewrite
synchronize_irq() by checking that the event queue is empty and waiting
for it to empty out otherwise. It's not safe enough to simply do the
power operations underneath the event_lock because there's a queue in
the kthread that might be waiting to grab the event_lock to process.
This sounds like a good reason to rewrite event_thread to use
threaded_irq and/or workqueue.
I think we are facing two problems,
1) event q is not empty after suspend (this scenario most likely will
not happen since display is off already) -- anyway it should be fixed
by adding "suspended" flag checking
2) new events add after suspend due to irq mask bits were not cleared
(this scenario most likely the major culprit) -- this fixed by remove
"enable" flag check at dp_ctrl_reset_irq_ctrl().
I will have "suspended" flag protected by event_lock.
Re: [PATCH v4 11/15] drm/shmem-helper: Add generic memory shrinker
On 5/12/22 20:04, Daniel Vetter wrote: > On Thu, 12 May 2022 at 13:36, Dmitry Osipenko > wrote: >> >> On 5/11/22 22:09, Daniel Vetter wrote: >>> On Wed, May 11, 2022 at 07:06:18PM +0300, Dmitry Osipenko wrote: On 5/11/22 16:09, Daniel Vetter wrote: > I'd like to ask you to reduce the scope of the patchset and build the > shrinker only for virtio-gpu. I know that I first suggested to build > upon shmem helpers, but it seems that it's easier to do that in a > later > patchset. The first version of the VirtIO shrinker didn't support memory eviction. Memory eviction support requires page fault handler to be aware of the evicted pages, what should we do about it? The page fault handling is a part of memory management, hence to me drm-shmem is already kinda a MM. >>> Hm I still don't get that part, why does that also not go through the >>> shmem helpers? >> The drm_gem_shmem_vm_ops includes the page faults handling, it's a >> helper by itself that is used by DRM drivers. >> >> I could try to move all the shrinker logic to the VirtIO and re-invent >> virtio_gem_shmem_vm_ops, but what is the point of doing this for each >> driver if we could have it once and for all in the common drm-shmem code? >> >> Maybe I should try to factor out all the shrinker logic from drm-shmem >> into a new drm-shmem-shrinker that could be shared by drivers? Will you >> be okay with this option? > I think we're talking past each another a bit. I'm only bringing up the > purge vs eviction topic we discussed in the other subthread again. Thomas asked to move the whole shrinker code to the VirtIO driver and I's saying that this is not a great idea to me, or am I misunderstanding the Thomas' suggestion? Thomas? >>> >>> I think it was just me creating a confusion here. >>> >>> fwiw I do also think that shrinker in shmem helpers makes sense, just in >>> case that was also lost in confusion. >> >> Okay, good that we're on the same page now. >> >>> I'm still confused why drivers need to know the difference >>> between evition and purging. Or maybe I'm confused again. >> Example: >> >> If userspace uses IOV addresses, then these addresses must be kept >> reserved while buffer is evicted. >> >> If BO is purged, then we don't need to retain the IOV space allocated >> for the purged BO. > Yeah but is that actually needed by anyone? If userspace fails to allocate > another bo because of lack of gpu address space then it's very easy to > handle that: > > 1. Make a rule that "out of gpu address space" gives you a special errno > code like ENOSPC > > 2. If userspace gets that it walks the list of all buffers it marked as > purgeable and nukes them (whether they have been evicted or not). Then it > retries the bo allocation. > > Alternatively you can do step 2 also directly from the bo alloc ioctl in > step 1. Either way you clean up va space, and actually a lot more (you > potentially nuke all buffers marked as purgeable, not just the ones that > have been purged already) and only when va cleanup is actually needed > > Trying to solve this problem at eviction time otoh means: > - we have this difference between eviction and purging > - it's still not complete, you still need to glue step 2 above into your > driver somehow, and once step 2 above is glued in doing additional > cleanup in the purge function is just duplicated logic > > So at least in my opinion this isn't the justification we need. And we > should definitely not just add that complication "in case, for the > future", if we don't have a real need right now. Adding it later on is > easy, removing it later on because it just gets in the way and confuses is > much harder. The IOVA space is only one example. In case of the VirtIO driver, we may have two memory allocation for a BO. One is the shmem allcation in guest and the other is in host's vram. If we will only release the guest's memory on purge, then the vram will remain allocated until BO is destroyed, which unnecessarily sub-optimal. >>> >>> Hm but why don't you just nuke the memory on the host side too when you >>> evict? Allowing the guest memory to be swapped out while keeping the host >>> memory allocation alive also doesn't make a lot of sense for me. Both can >>> be recreated (I guess at least?) on swap-in. >> >> Shouldn't be very doable or at least worth the efforts. It's userspace >> that manages data uploading, kernel only provides transport for the >> virtio-gpu commands. >> >> Drivers are free to use the same function for both purge() and evict() >> callbacks if they want. Getting rid of the purge() callback creates more >> problems than solves, IMO. > > Hm this still sounds
Re: [PATCH v1 01/15] mm: add zone device coherent type memory support
On 5/11/2022 9:58 PM, Alistair Popple wrote:
Alex Sierra writes:
[...]
diff --git a/mm/rmap.c b/mm/rmap.c
index fedb82371efe..d57102cd4b43 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1995,7 +1995,8 @@ void try_to_migrate(struct folio *folio, enum ttu_flags
flags)
TTU_SYNC)))
return;
- if (folio_is_zone_device(folio) && !folio_is_device_private(folio))
+ if (folio_is_zone_device(folio) &&
+ (!folio_is_device_private(folio) &&
!folio_is_device_coherent(folio)))
return;
/*
I vaguely recall commenting on this previously, or at least intending
to. In try_to_migrate_one() we have this:
if (folio_is_zone_device(folio)) {
unsigned long pfn = folio_pfn(folio);
swp_entry_t entry;
pte_t swp_pte;
/*
* Store the pfn of the page in a special migration
* pte. do_swap_page() will wait until the migration
* pte is removed and then restart fault handling.
*/
entry = pte_to_swp_entry(pteval);
if (is_writable_device_private_entry(entry))
entry = make_writable_migration_entry(pfn);
else
entry = make_readable_migration_entry(pfn);
swp_pte = swp_entry_to_pte(entry);
The check in try_to_migrate() guarantees that if folio_is_zone_device()
is true this must be a DEVICE_PRIVATE page and it treats it as such by
assuming there is a special device private swap entry there.
Relying on that assumption seems bad, and I have no idea why I didn't
just use is_device_private_page() originally but I think the fix is just
to change this to:
if (folio_is_device_private(folio))
Thanks Alistair. This worked fine. I'll drop patch 4 and update this
patch in the next series version.
Regards,
Alex Sierra
And let DEVICE_COHERENT pages fall through to normal page processing.
- Alistair
Re: [PATCH v5 7/7] fbdev: Make registered_fb[] private to fbmem.c
On Wed, May 11, 2022 at 07:34:38PM +0200, Javier Martinez Canillas wrote: > Hello Guenter, > > On 5/11/22 19:17, Guenter Roeck wrote: > > On 5/11/22 10:00, Sam Ravnborg wrote: > > [snip] > > >>> struct fb_info *registered_fb[FB_MAX] __read_mostly; > >>> -EXPORT_SYMBOL(registered_fb); > >>> - > >>> int num_registered_fb __read_mostly; > >>> +#if IS_ENABLED(CONFIG_FB_OLPC_DCON) > >>> +EXPORT_SYMBOL(registered_fb); > >>> EXPORT_SYMBOL(num_registered_fb); > >>> +#endif > >> > >> It is stuff like this I refer to as "ugly" in the comment above. > >> > > > > My "solution" for that kind of thing is to use a namespace, > > such as > > > > EXPORT_SYMBOL_NS(registered_fb, FB_OLPC_DCON); > > EXPORT_SYMBOL_NS(num_registered_fb, FB_OLPC_DCON); > > > > Using a namespace in this case is indeed a great idea I think. > > I've used in the past to limit the export of a symbol for within a driver > that could be scattered across different compilations units, but it never > occurred to me using it to limit symbols exported by core code. > > > and import it from the offending code. That avoids ifdefs > > while at the same time limiting the use of the symbols > > to the expected scope. Of course that could be abused but > > that abuse would be obvious. > > > > Agreed. For the next revision, besides using an namespaced export symbol > as you suggested, I'll include a comment to make clear that it shouldn't > by any other driver and FB_OLPC_DCON fixed instead. A very nice compromise, thanks Guenter and Javier. Sam
Re: [PATCH v4 0/7] Make the rest of the VFIO driver interface use vfio_device
On Thu, 5 May 2022 21:08:38 -0300 Jason Gunthorpe wrote: > Prior series have transformed other parts of VFIO from working on struct > device or struct vfio_group into working directly on struct > vfio_device. Based on that work we now have vfio_device's readily > available in all the drivers. > > Update the rest of the driver facing API to use vfio_device as an input. > > The following are switched from struct device to struct vfio_device: > vfio_register_notifier() > vfio_unregister_notifier() > vfio_pin_pages() > vfio_unpin_pages() > vfio_dma_rw() > > The following group APIs are obsoleted and removed by just using struct > vfio_device with the above: > vfio_group_pin_pages() > vfio_group_unpin_pages() > vfio_group_iommu_domain() > vfio_group_get_external_user_from_dev() > > To retain the performance of the new device APIs relative to their group > versions optimize how vfio_group_add_container_user() is used to avoid > calling it when the driver must already guarantee the device is open and > the container_users incrd. > > The remaining exported VFIO group interfaces are only used by kvm, and are > addressed by a parallel series. > > This series is based on Christoph's gvt rework here: > > https://lore.kernel.org/all/5a8b9f48-2c32-8177-1c18-e3bd7bfde...@intel.com/ > > and so will need the PR merged first. > > I have a followup series that needs this. > > This is also part of the iommufd work - moving the driver facing interface > to vfio_device provides a much cleaner path to integrate with iommufd. > > v4: > - Use 'device' as the argument name for a struct vfio_device in vfio.c > v3: > https://lore.kernel.org/r/0-v3-e131a9b6b467+14b6-vfio_mdev_no_group_...@nvidia.com > - Based on VFIO's gvt/iommu merge > - Remove mention of mdev_legacy_get_vfio_device() from commit message > - Clarify commit message for vfio_dma_rw() conversion > - Talk about the open_count change in the commit message > - No code change > v2: > https://lore.kernel.org/r/0-v2-6011bde8e0a1+5f-vfio_mdev_no_group_...@nvidia.com > - Based on Christoph's series so mdev_legacy_get_vfio_device() is removed > - Reflow indenting > - Use vfio_assert_device_open() and WARN_ON_ONCE instead of open coding >the assertion > v1: > https://lore.kernel.org/r/0-v1-a8faf768d202+125dd-vfio_mdev_no_group_...@nvidia.com > > Jason Gunthorpe (7): > vfio: Make vfio_(un)register_notifier accept a vfio_device > vfio/ccw: Remove mdev from struct channel_program > vfio/mdev: Pass in a struct vfio_device * to vfio_pin/unpin_pages() > vfio/mdev: Pass in a struct vfio_device * to vfio_dma_rw() > drm/i915/gvt: Change from vfio_group_(un)pin_pages to > vfio_(un)pin_pages > vfio: Remove dead code > vfio: Remove calls to vfio_group_add_container_user() > > .../driver-api/vfio-mediated-device.rst | 4 +- > drivers/gpu/drm/i915/gvt/gvt.h| 5 +- > drivers/gpu/drm/i915/gvt/kvmgt.c | 51 ++- > drivers/s390/cio/vfio_ccw_cp.c| 47 +-- > drivers/s390/cio/vfio_ccw_cp.h| 4 +- > drivers/s390/cio/vfio_ccw_fsm.c | 3 +- > drivers/s390/cio/vfio_ccw_ops.c | 7 +- > drivers/s390/crypto/vfio_ap_ops.c | 23 +- > drivers/vfio/vfio.c | 299 +++--- > include/linux/vfio.h | 21 +- > 10 files changed, 109 insertions(+), 355 deletions(-) Applied to vfio next branch for v5.19. Thanks, Alex
Re: [BUG] Warning and NULL-ptr dereference in amdgpu driver with 5.18
On Thu, May 12, 2022 at 09:47:29AM -0400, Alex Deucher wrote: > Are those new? Maybe the card is not seated correctly? Can you try > another slot? I can't remember having seen these TLP error messages with older kernels. 5.17 still works fine with this card. I will try to put the card into another slot tomorrow. > As for the null pointer defer in the display code, @Wentland, Harry > any ideas? I don't see why that should happen. Maybe some hotplug > pin is faulty or the display has input detection and that is causing > some sort of hotplug interrupt that causes a race somewhere in the > driver? Can you make sure the monitor connector is firmly seated on > the GPU? The connectors are fine, the displays are connected via miniDP on the GPU and DP on the display side. On the other hand my monitors do not seem to have the highest quality. Occassionally the resolution is wrongly detected or DP signal is lost. I am not sure why, I suspect there is some interference between the two DP cables. But this is a problem for as long as I have these two monitors, the NULL-ptr deref only happens with v5.18. Regards, -- Jörg Rödel jroe...@suse.de SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Re: [PATCH] drm/amd/display: Remove macro DC_DEFAULT_LOG_MASK
Hi pengfuyuan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm/drm-next]
[also build test ERROR on v5.18-rc6]
[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/intel-lab-lkp/linux/commits/pengfuyuan/drm-amd-display-Remove-macro-DC_DEFAULT_LOG_MASK/20220512-185320
base: git://anongit.freedesktop.org/drm/drm drm-next
config: powerpc64-randconfig-s032-20220512
(https://download.01.org/0day-ci/archive/20220513/202205130102.up3i0eb0-...@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 11.3.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
#
https://github.com/intel-lab-lkp/linux/commit/94b3092ea272cf77105cc7b19fcffc44b49e1a71
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
pengfuyuan/drm-amd-display-Remove-macro-DC_DEFAULT_LOG_MASK/20220512-185320
git checkout 94b3092ea272cf77105cc7b19fcffc44b49e1a71
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=powerpc
SHELL=/bin/bash
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 drivers/gpu/drm/amd/amdgpu/../display/dc/dc.h:31,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:30:
>> drivers/gpu/drm/amd/amdgpu/../display/include/logger_types.h:26: error:
>> unterminated #ifndef
26 | #ifndef __DAL_LOGGER_TYPES_H__
|
In file included from
drivers/gpu/drm/amd/amdgpu/../display/include/logger_interface.h:29,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:36,
from
drivers/gpu/drm/amd/amdgpu/../display/include/bios_parser_types.h:30,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/inc/clock_source.h:31,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/inc/hw_sequencer.h:29,
from drivers/gpu/drm/amd/amdgpu/../display/dc/dc.h:40,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:30:
>> drivers/gpu/drm/amd/amdgpu/../display/include/logger_types.h:26: error:
>> unterminated #ifndef
26 | #ifndef __DAL_LOGGER_TYPES_H__
|
In file included from
drivers/gpu/drm/amd/amdgpu/../display/dc/inc/core_types.h:32,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/inc/link_enc_cfg.h:33,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:32:
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:131:22:
warning: 'SYNAPTICS_DEVICE_ID' defined but not used [-Wunused-const-variable=]
131 | static const uint8_t SYNAPTICS_DEVICE_ID[] = "SYNA";
| ^~~
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:128:22:
warning: 'DP_SINK_DEVICE_STR_ID_2' defined but not used
[-Wunused-const-variable=]
128 | static const uint8_t DP_SINK_DEVICE_STR_ID_2[] = {7, 1, 8, 7, 5, 0};
| ^~~
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:127:22:
warning: 'DP_SINK_DEVICE_STR_ID_1' defined but not used
[-Wunused-const-variable=]
127 | static const uint8_t DP_SINK_DEVICE_STR_ID_1[] = {7, 1, 8, 7, 3, 0};
| ^~~
--
In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc.h:31,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c:27:
>> drivers/gpu/drm/amd/amdgpu/../display/include/logger_types.h:26: error:
>> unterminated #ifndef
26 | #ifndef __DAL_LOGGER_TYPES_H__
|
In file included from
drivers/gpu/drm/amd/amdgpu/../display/include/logger_interface.h:29,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:36,
from
drivers/gpu/drm/amd/amdgpu/../display/include/bios_parser_types.h:30,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/inc/clock_source.h:31,
from
drivers/gpu/drm/amd/amdgpu/../display/dc/inc/hw_sequencer.h:29,
from drivers/gpu/drm/amd/amdgpu/../display/dc/dc.h:40,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_irq.c:27:
>> drivers/gpu/drm/amd/amdgpu/../display/include/logger_types.h:26: error:
Re: [PATCH v4 11/15] drm/shmem-helper: Add generic memory shrinker
On Thu, 12 May 2022 at 13:36, Dmitry Osipenko wrote: > > On 5/11/22 22:09, Daniel Vetter wrote: > > On Wed, May 11, 2022 at 07:06:18PM +0300, Dmitry Osipenko wrote: > >> On 5/11/22 16:09, Daniel Vetter wrote: > >>> I'd like to ask you to reduce the scope of the patchset and build the > >>> shrinker only for virtio-gpu. I know that I first suggested to build > >>> upon shmem helpers, but it seems that it's easier to do that in a > >>> later > >>> patchset. > >> The first version of the VirtIO shrinker didn't support memory > >> eviction. > >> Memory eviction support requires page fault handler to be aware of the > >> evicted pages, what should we do about it? The page fault handling is a > >> part of memory management, hence to me drm-shmem is already kinda a MM. > > Hm I still don't get that part, why does that also not go through the > > shmem helpers? > The drm_gem_shmem_vm_ops includes the page faults handling, it's a > helper by itself that is used by DRM drivers. > > I could try to move all the shrinker logic to the VirtIO and re-invent > virtio_gem_shmem_vm_ops, but what is the point of doing this for each > driver if we could have it once and for all in the common drm-shmem code? > > Maybe I should try to factor out all the shrinker logic from drm-shmem > into a new drm-shmem-shrinker that could be shared by drivers? Will you > be okay with this option? > >>> I think we're talking past each another a bit. I'm only bringing up the > >>> purge vs eviction topic we discussed in the other subthread again. > >> > >> Thomas asked to move the whole shrinker code to the VirtIO driver and > >> I's saying that this is not a great idea to me, or am I misunderstanding > >> the Thomas' suggestion? Thomas? > > > > I think it was just me creating a confusion here. > > > > fwiw I do also think that shrinker in shmem helpers makes sense, just in > > case that was also lost in confusion. > > Okay, good that we're on the same page now. > > > I'm still confused why drivers need to know the difference > > between evition and purging. Or maybe I'm confused again. > Example: > > If userspace uses IOV addresses, then these addresses must be kept > reserved while buffer is evicted. > > If BO is purged, then we don't need to retain the IOV space allocated > for the purged BO. > >>> Yeah but is that actually needed by anyone? If userspace fails to allocate > >>> another bo because of lack of gpu address space then it's very easy to > >>> handle that: > >>> > >>> 1. Make a rule that "out of gpu address space" gives you a special errno > >>> code like ENOSPC > >>> > >>> 2. If userspace gets that it walks the list of all buffers it marked as > >>> purgeable and nukes them (whether they have been evicted or not). Then it > >>> retries the bo allocation. > >>> > >>> Alternatively you can do step 2 also directly from the bo alloc ioctl in > >>> step 1. Either way you clean up va space, and actually a lot more (you > >>> potentially nuke all buffers marked as purgeable, not just the ones that > >>> have been purged already) and only when va cleanup is actually needed > >>> > >>> Trying to solve this problem at eviction time otoh means: > >>> - we have this difference between eviction and purging > >>> - it's still not complete, you still need to glue step 2 above into your > >>> driver somehow, and once step 2 above is glued in doing additional > >>> cleanup in the purge function is just duplicated logic > >>> > >>> So at least in my opinion this isn't the justification we need. And we > >>> should definitely not just add that complication "in case, for the > >>> future", if we don't have a real need right now. Adding it later on is > >>> easy, removing it later on because it just gets in the way and confuses is > >>> much harder. > >> > >> The IOVA space is only one example. > >> > >> In case of the VirtIO driver, we may have two memory allocation for a > >> BO. One is the shmem allcation in guest and the other is in host's vram. > >> If we will only release the guest's memory on purge, then the vram will > >> remain allocated until BO is destroyed, which unnecessarily sub-optimal. > > > > Hm but why don't you just nuke the memory on the host side too when you > > evict? Allowing the guest memory to be swapped out while keeping the host > > memory allocation alive also doesn't make a lot of sense for me. Both can > > be recreated (I guess at least?) on swap-in. > > Shouldn't be very doable or at least worth the efforts. It's userspace > that manages data uploading, kernel only provides transport for the > virtio-gpu commands. > > Drivers are free to use the same function for both purge() and evict() > callbacks if they want. Getting rid of the purge() callback creates more > problems than solves, IMO. Hm this still sounds pretty funny and defeats the point of purgeable/evictable
[PATCH] drm/amd/display: Remove macro DC_DEFAULT_LOG_MASK
[Why & How]
The DC_DEFAULT_LOG_MASK macro has not been used for a long time, so remove it.
Signed-off-by: pengfuyuan
---
.../drm/amd/display/include/logger_types.h| 34 ---
1 file changed, 34 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/include/logger_types.h
b/drivers/gpu/drm/amd/display/include/logger_types.h
index f093b49c5e6e..a31d7c959f2c 100644
--- a/drivers/gpu/drm/amd/display/include/logger_types.h
+++ b/drivers/gpu/drm/amd/display/include/logger_types.h
@@ -131,37 +131,3 @@ enum dc_log_type {
#define DC_MIN_LOG_MASK ((1 << LOG_ERROR) | \
(1 << LOG_DETECTION_EDID_PARSER))
-#define DC_DEFAULT_LOG_MASK ((1ULL << LOG_ERROR) | \
- (1ULL << LOG_WARNING) | \
- (1ULL << LOG_EVENT_MODE_SET) | \
- (1ULL << LOG_EVENT_DETECTION) | \
- (1ULL << LOG_EVENT_LINK_TRAINING) | \
- (1ULL << LOG_EVENT_LINK_LOSS) | \
- (1ULL << LOG_EVENT_UNDERFLOW) | \
- (1ULL << LOG_RESOURCE) | \
- (1ULL << LOG_FEATURE_OVERRIDE) | \
- (1ULL << LOG_DETECTION_EDID_PARSER) | \
- (1ULL << LOG_DC) | \
- (1ULL << LOG_HW_HOTPLUG) | \
- (1ULL << LOG_HW_SET_MODE) | \
- (1ULL << LOG_HW_RESUME_S3) | \
- (1ULL << LOG_HW_HPD_IRQ) | \
- (1ULL << LOG_SYNC) | \
- (1ULL << LOG_BANDWIDTH_VALIDATION) | \
- (1ULL << LOG_MST) | \
- (1ULL << LOG_DETECTION_DP_CAPS) | \
- (1ULL << LOG_BACKLIGHT)) | \
- (1ULL << LOG_I2C_AUX) | \
- (1ULL << LOG_IF_TRACE) | \
- (1ULL << LOG_HDMI_FRL) | \
- (1ULL << LOG_SCALER) | \
- (1ULL << LOG_DTN) /* | \
- (1ULL << LOG_DEBUG) | \
- (1ULL << LOG_BIOS) | \
- (1ULL << LOG_SURFACE) | \
- (1ULL << LOG_DML) | \
- (1ULL << LOG_HW_LINK_TRAINING) | \
- (1ULL << LOG_HW_AUDIO)| \
- (1ULL << LOG_BANDWIDTH_CALCS)*/
-
-#endif /* __DAL_LOGGER_TYPES_H__ */
--
2.25.1
Re: [PATCH] drm/i915: Use i915_gem_object_ggtt_pin_ww for reloc_iomap
On 11/05/2022 19:38, Maarten Lankhorst wrote:
Op 11-05-2022 om 20:23 schreef Matthew Auld:
On 11/05/2022 12:52, Maarten Lankhorst wrote:
Instead of its own path, use the common path when it doesn't result
in evicting any vma. This fixes the case where we don't wait for
binding.
https://gitlab.freedesktop.org/drm/intel/-/issues/5806
If I'm reading that correctly waiting for the bind doesn't seem to help?
I suspect the actual pinning there might do some stuff that we are not doing.
It was working before the change, and manually calling pin caused the failure,
so I reverted it back to what was working before. It was specifically the
manual pin code that was failing.
I can change the commit message if it helps.
Hmm strange. With the commit message updated,
Acked-by: Matthew Auld
~Maarten
Fixes: b5cfe6f7a6e1 ("drm/i915: Remove short-term pins from execbuf, v6.")
Cc: Matthew Auld
Reported-by: Mateusz Jończyk
Tested-by: Hans de Goede
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 498b458fd784..919d01082909 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1262,14 +1262,12 @@ static void *reloc_iomap(struct i915_vma *batch,
* Only attempt to pin the batch buffer to ggtt if the current batch
* is not inside ggtt, or the batch buffer is not misplaced.
*/
- if (!i915_is_ggtt(batch->vm)) {
+ if (!i915_is_ggtt(batch->vm) ||
+ !i915_vma_misplaced(batch, 0, 0, PIN_MAPPABLE)) {
vma = i915_gem_object_ggtt_pin_ww(obj, >ww, NULL, 0, 0,
PIN_MAPPABLE |
PIN_NONBLOCK /* NOWARN */ |
PIN_NOEVICT);
- } else if (i915_vma_is_map_and_fenceable(batch)) {
- __i915_vma_pin(batch);
- vma = batch;
}
if (vma == ERR_PTR(-EDEADLK))
Re: [REPORT] syscall reboot + umh + firmware fallback
Hello, On Thu, May 12, 2022 at 08:18:24PM +0900, Byungchul Park wrote: > > 1. wait_for_completion_killable_timeout() doesn't need someone to wake it up > >to make forward progress because it will unstick itself after timeout > >expires. > > I have a question about this one. Yes, it would never been stuck thanks > to timeout. However, IIUC, timeouts are not supposed to expire in normal > cases. So I thought a timeout expiration means not a normal case so need > to inform it in terms of dependency so as to prevent further expiraton. > That's why I have been trying to track even timeout'ed APIs. > > Do you think DEPT shouldn't track timeout APIs? If I was wrong, I > shouldn't track the timeout APIs any more. Without actually surveying the use cases, I can't say for sure but my experience has been that we often get pretty creative with timeouts and it's something people actively think about and monitor (and it's usually not subtle). Given that, I'm skeptical about how much value it'd add for a dependency checker to warn about timeouts. It might be net negative than the other way around. > > 2. complete_all() from __fw_load_abort() isn't the only source of wakeup. > >The fw loader can be, and mainly should be, woken up by firmware loading > >actually completing instead of being aborted. > > This is the point I'd like to ask. In normal cases, fw_load_done() might > happen, of course, if the loading gets completed. However, I was > wondering if the kernel ensures either fw_load_done() or fw_load_abort() > to be called by *another* context while kernel_halt(). We'll have to walk through the code to tell that. On a cursory look tho, up until that point (just before shutting down usermode helper), I don't see anything which would actively block firmware loading. Thanks. -- tejun
Re: [PATCH 1/2] drm/bridge: tc358767: Factor out DSI and DPI RX enablement
On Thu, 12 May 2022 at 18:02, Robert Foss wrote:
>
> On Fri, 29 Apr 2022 at 22:56, Marek Vasut wrote:
> >
> > Factor out register programming to configure the chip video RX side for
> > reception of video data from DSI or DPI. This is particularly useful in
> > the (e)DP output mode, where the video data can be received from either
> > DPI or DSI. While only the former is supported in (e)DP output mode so
> > far, this patch is added in preparation for addition of the later.
> >
> > There is a change in the order or register programming in case of the
> > DSI-to-DPI mode. The DSI RX side is now programmed and enabled all in
> > one place after the output mode has been configured. Before this change,
> > the DSI RX has been programmed before the output mode has been set and
> > only enabled afterward. The order makes no difference however, since the
> > DSI RX is only enabled at the end either way.
> >
> > Signed-off-by: Marek Vasut
> > Cc: Jonas Karlman
> > Cc: Laurent Pinchart
> > Cc: Lucas Stach
> > Cc: Marek Vasut
> > Cc: Maxime Ripard
> > Cc: Neil Armstrong
> > Cc: Robert Foss
> > Cc: Sam Ravnborg
> > ---
> > drivers/gpu/drm/bridge/tc358767.c | 94 +--
> > 1 file changed, 53 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/tc358767.c
> > b/drivers/gpu/drm/bridge/tc358767.c
> > index 485717c8f0b4..e72dd5cd9700 100644
> > --- a/drivers/gpu/drm/bridge/tc358767.c
> > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > @@ -1247,11 +1247,60 @@ static int tc_main_link_disable(struct tc_data *tc)
> > return regmap_write(tc->regmap, DP0CTL, 0);
> > }
> >
> > -static int tc_dpi_stream_enable(struct tc_data *tc)
> > +static int tc_dsi_rx_enable(struct tc_data *tc)
> > {
> > + u32 value;
> > int ret;
> > +
> > + regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
> > + regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
> > + regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
> > + regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
> > + regmap_write(tc->regmap, PPI_D0S_ATMR, 0);
> > + regmap_write(tc->regmap, PPI_D1S_ATMR, 0);
> > + regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> > + regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
> > +
> > + value = ((LANEENABLE_L0EN << tc->dsi_lanes) - LANEENABLE_L0EN) |
> > + LANEENABLE_CLEN;
> > + regmap_write(tc->regmap, PPI_LANEENABLE, value);
> > + regmap_write(tc->regmap, DSI_LANEENABLE, value);
> > +
> > + /* Set input interface */
> > + value = DP0_AUDSRC_NO_INPUT;
> > + if (tc_test_pattern)
> > + value |= DP0_VIDSRC_COLOR_BAR;
> > + else
> > + value |= DP0_VIDSRC_DSI_RX;
> > + ret = regmap_write(tc->regmap, SYSCTRL, value);
> > + if (ret)
> > + return ret;
> > +
> > + usleep_range(120, 150);
> > +
> > + regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
> > + regmap_write(tc->regmap, DSI_STARTDSI, DSI_RX_START);
> > +
> > + return 0;
> > +}
> > +
> > +static int tc_dpi_rx_enable(struct tc_data *tc)
> > +{
> > u32 value;
> >
> > + /* Set input interface */
> > + value = DP0_AUDSRC_NO_INPUT;
> > + if (tc_test_pattern)
> > + value |= DP0_VIDSRC_COLOR_BAR;
> > + else
> > + value |= DP0_VIDSRC_DPI_RX;
> > + return regmap_write(tc->regmap, SYSCTRL, value);
> > +}
> > +
> > +static int tc_dpi_stream_enable(struct tc_data *tc)
> > +{
> > + int ret;
> > +
> > dev_dbg(tc->dev, "enable video stream\n");
> >
> > /* Setup PLL */
> > @@ -1277,20 +1326,6 @@ static int tc_dpi_stream_enable(struct tc_data *tc)
> > if (ret)
> > return ret;
> >
> > - regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
> > - regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
> > - regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
> > - regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
> > - regmap_write(tc->regmap, PPI_D0S_ATMR, 0);
> > - regmap_write(tc->regmap, PPI_D1S_ATMR, 0);
> > - regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> > - regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
> > -
> > - value = ((LANEENABLE_L0EN << tc->dsi_lanes) - LANEENABLE_L0EN) |
> > - LANEENABLE_CLEN;
> > - regmap_write(tc->regmap, PPI_LANEENABLE, value);
> > - regmap_write(tc->regmap, DSI_LANEENABLE, value);
> > -
> > ret = tc_set_common_video_mode(tc, >mode);
> > if (ret)
> > return ret;
> > @@ -1299,22 +1334,7 @@ static int tc_dpi_stream_enable(struct tc_data *tc)
> > if (ret)
> > return ret;
> >
> > - /* Set input interface */
> > - value = DP0_AUDSRC_NO_INPUT;
> > - if (tc_test_pattern)
> > - value |= DP0_VIDSRC_COLOR_BAR;
> > -
Re: [PATCH 2/2] drm/bridge: tc358767: Add DSI-to-(e)DP mode support
On Fri, 29 Apr 2022 at 22:56, Marek Vasut wrote:
>
> Implement DSI-to-e(DP) mode, which is a mix of currently supported
> DSI-to-DPI and DPI-to-(e)DP modes. The input side is configured as
> either DSI or DPI, the DP AUX channel is registered for both input
> side options, and the DSI host is attached for both DPI and (e)DP
> output side options.
>
> One notable detail is that the DSI-to-(e)DP mode requires the Pixel
> PLL to be always enabled, which is not needed for DPI-to-(e)DP mode
> which gets the matching clock direct from DPI Pixel Clock instead.
>
> Signed-off-by: Marek Vasut
> Cc: Jonas Karlman
> Cc: Laurent Pinchart
> Cc: Lucas Stach
> Cc: Marek Vasut
> Cc: Maxime Ripard
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Sam Ravnborg
> ---
> drivers/gpu/drm/bridge/tc358767.c | 40 +++
> 1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index e72dd5cd9700..798da0e4d086 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -309,6 +309,9 @@ struct tc_data {
> /* do we have IRQ */
> boolhave_irq;
>
> + /* Input connector type, DSI and not DPI. */
> + boolinput_connector_dsi;
> +
> /* HPD pin number (0 or 1) or -ENODEV */
> int hpd_pin;
> };
> @@ -1353,8 +1356,18 @@ static int tc_edp_stream_enable(struct tc_data *tc)
>
> dev_dbg(tc->dev, "enable video stream\n");
>
> - /* PXL PLL setup */
> - if (tc_test_pattern) {
> + /*
> +* Pixel PLL must be enabled for DSI input mode and test pattern.
> +*
> +* Per TC9595XBG datasheet Revision 0.1 2018-12-27 Figure 4.18
> +* "Clock Mode Selection and Clock Sources", either Pixel PLL
> +* or DPI_PCLK supplies StrmClk. DPI_PCLK is only available in
> +* case valid Pixel Clock are supplied to the chip DPI input.
> +* In case built-in test pattern is desired OR DSI input mode
> +* is used, DPI_PCLK is not available and thus Pixel PLL must
> +* be used instead.
> +*/
> + if (tc->input_connector_dsi || tc_test_pattern) {
> ret = tc_pxl_pll_en(tc, clk_get_rate(tc->refclk),
> 1000 * tc->mode.clock);
> if (ret)
> @@ -1394,7 +1407,10 @@ static int tc_edp_stream_enable(struct tc_data *tc)
> return ret;
>
> /* Set input interface */
> - return tc_dpi_rx_enable(tc);
> + if (tc->input_connector_dsi)
> + return tc_dsi_rx_enable(tc);
> + else
> + return tc_dpi_rx_enable(tc);
> }
>
> static int tc_edp_stream_disable(struct tc_data *tc)
> @@ -2004,14 +2020,18 @@ static int tc_probe_bridge_endpoint(struct tc_data
> *tc)
> mode |= BIT(endpoint.port);
> }
>
> - if (mode == mode_dpi_to_edp || mode == mode_dpi_to_dp)
> + if (mode == mode_dpi_to_edp || mode == mode_dpi_to_dp) {
> + tc->input_connector_dsi = false;
> return tc_probe_edp_bridge_endpoint(tc);
> - else if (mode == mode_dsi_to_dpi)
> + } else if (mode == mode_dsi_to_dpi) {
> + tc->input_connector_dsi = true;
> return tc_probe_dpi_bridge_endpoint(tc);
> - else if (mode == mode_dsi_to_edp || mode == mode_dsi_to_dp)
> - dev_warn(dev, "The mode DSI-to-(e)DP is not supported!\n");
> - else
> - dev_warn(dev, "Invalid mode (0x%x) is not supported!\n",
> mode);
> + } else if (mode == mode_dsi_to_edp || mode == mode_dsi_to_dp) {
> + tc->input_connector_dsi = true;
> + return tc_probe_edp_bridge_endpoint(tc);
> + }
> +
> + dev_warn(dev, "Invalid mode (0x%x) is not supported!\n", mode);
>
> return -EINVAL;
> }
> @@ -2149,7 +2169,7 @@ static int tc_probe(struct i2c_client *client, const
> struct i2c_device_id *id)
>
> i2c_set_clientdata(client, tc);
>
> - if (tc->bridge.type == DRM_MODE_CONNECTOR_DPI) { /* DPI output */
> + if (tc->input_connector_dsi) { /* DSI input */
> ret = tc_mipi_dsi_host_attach(tc);
> if (ret) {
> drm_bridge_remove(>bridge);
Reviewed-by: Robert Foss
Re: [PATCH 1/2] drm/bridge: tc358767: Factor out DSI and DPI RX enablement
On Fri, 29 Apr 2022 at 22:56, Marek Vasut wrote:
>
> Factor out register programming to configure the chip video RX side for
> reception of video data from DSI or DPI. This is particularly useful in
> the (e)DP output mode, where the video data can be received from either
> DPI or DSI. While only the former is supported in (e)DP output mode so
> far, this patch is added in preparation for addition of the later.
>
> There is a change in the order or register programming in case of the
> DSI-to-DPI mode. The DSI RX side is now programmed and enabled all in
> one place after the output mode has been configured. Before this change,
> the DSI RX has been programmed before the output mode has been set and
> only enabled afterward. The order makes no difference however, since the
> DSI RX is only enabled at the end either way.
>
> Signed-off-by: Marek Vasut
> Cc: Jonas Karlman
> Cc: Laurent Pinchart
> Cc: Lucas Stach
> Cc: Marek Vasut
> Cc: Maxime Ripard
> Cc: Neil Armstrong
> Cc: Robert Foss
> Cc: Sam Ravnborg
> ---
> drivers/gpu/drm/bridge/tc358767.c | 94 +--
> 1 file changed, 53 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c
> b/drivers/gpu/drm/bridge/tc358767.c
> index 485717c8f0b4..e72dd5cd9700 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -1247,11 +1247,60 @@ static int tc_main_link_disable(struct tc_data *tc)
> return regmap_write(tc->regmap, DP0CTL, 0);
> }
>
> -static int tc_dpi_stream_enable(struct tc_data *tc)
> +static int tc_dsi_rx_enable(struct tc_data *tc)
> {
> + u32 value;
> int ret;
> +
> + regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
> + regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
> + regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
> + regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
> + regmap_write(tc->regmap, PPI_D0S_ATMR, 0);
> + regmap_write(tc->regmap, PPI_D1S_ATMR, 0);
> + regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> + regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
> +
> + value = ((LANEENABLE_L0EN << tc->dsi_lanes) - LANEENABLE_L0EN) |
> + LANEENABLE_CLEN;
> + regmap_write(tc->regmap, PPI_LANEENABLE, value);
> + regmap_write(tc->regmap, DSI_LANEENABLE, value);
> +
> + /* Set input interface */
> + value = DP0_AUDSRC_NO_INPUT;
> + if (tc_test_pattern)
> + value |= DP0_VIDSRC_COLOR_BAR;
> + else
> + value |= DP0_VIDSRC_DSI_RX;
> + ret = regmap_write(tc->regmap, SYSCTRL, value);
> + if (ret)
> + return ret;
> +
> + usleep_range(120, 150);
> +
> + regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
> + regmap_write(tc->regmap, DSI_STARTDSI, DSI_RX_START);
> +
> + return 0;
> +}
> +
> +static int tc_dpi_rx_enable(struct tc_data *tc)
> +{
> u32 value;
>
> + /* Set input interface */
> + value = DP0_AUDSRC_NO_INPUT;
> + if (tc_test_pattern)
> + value |= DP0_VIDSRC_COLOR_BAR;
> + else
> + value |= DP0_VIDSRC_DPI_RX;
> + return regmap_write(tc->regmap, SYSCTRL, value);
> +}
> +
> +static int tc_dpi_stream_enable(struct tc_data *tc)
> +{
> + int ret;
> +
> dev_dbg(tc->dev, "enable video stream\n");
>
> /* Setup PLL */
> @@ -1277,20 +1326,6 @@ static int tc_dpi_stream_enable(struct tc_data *tc)
> if (ret)
> return ret;
>
> - regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
> - regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
> - regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
> - regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
> - regmap_write(tc->regmap, PPI_D0S_ATMR, 0);
> - regmap_write(tc->regmap, PPI_D1S_ATMR, 0);
> - regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> - regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
> -
> - value = ((LANEENABLE_L0EN << tc->dsi_lanes) - LANEENABLE_L0EN) |
> - LANEENABLE_CLEN;
> - regmap_write(tc->regmap, PPI_LANEENABLE, value);
> - regmap_write(tc->regmap, DSI_LANEENABLE, value);
> -
> ret = tc_set_common_video_mode(tc, >mode);
> if (ret)
> return ret;
> @@ -1299,22 +1334,7 @@ static int tc_dpi_stream_enable(struct tc_data *tc)
> if (ret)
> return ret;
>
> - /* Set input interface */
> - value = DP0_AUDSRC_NO_INPUT;
> - if (tc_test_pattern)
> - value |= DP0_VIDSRC_COLOR_BAR;
> - else
> - value |= DP0_VIDSRC_DSI_RX;
> - ret = regmap_write(tc->regmap, SYSCTRL, value);
> - if (ret)
> - return ret;
> -
> - usleep_range(120, 150);
> -
> - regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
> -
Re: [PATCH v5] drm/msm/dp: Always clear mask bits to disable interrupts at dp_ctrl_reset_irq_ctrl()
On 5/11/2022 6:03 PM, Dmitry Baryshkov wrote:
On Thu, 12 May 2022 at 04:01, Stephen Boyd wrote:
Quoting Dmitry Baryshkov (2022-05-11 17:41:50)
On 12/05/2022 03:02, Kuogee Hsieh wrote:
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index af7a80c..f3e333e 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1389,8 +1389,13 @@ void dp_ctrl_reset_irq_ctrl(struct dp_ctrl *dp_ctrl,
bool enable)
dp_catalog_ctrl_reset(ctrl->catalog);
- if (enable)
- dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
+ /*
+ * all dp controller programmable registers will not
+ * be reset to default value after DP_SW_RESET
+ * therefore interrupt mask bits have to be updated
+ * to enable/disable interrupts
+ */
+ dp_catalog_ctrl_enable_irq(ctrl->catalog, enable);
}
void dp_ctrl_phy_init(struct dp_ctrl *dp_ctrl)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index c388323..79439b8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -98,6 +98,8 @@ struct dp_display_private {
struct dp_ctrl*ctrl;
struct dp_debug *debug;
+ atomic_t suspended;
I think it'd be better to protect it with event_lock rather than using
atomics.
Agreed. I think the concern is that the event queue will have "stuff" in
it. If the event queue was all a threaded irq we could simply call
synchronize_irq() after disabling the irq bit in the DP hardware and
then we would know it is safe to power down the DP logic. Unfortunately
the event queue is a kthread so we can't do that and we have to rewrite
synchronize_irq() by checking that the event queue is empty and waiting
for it to empty out otherwise. It's not safe enough to simply do the
power operations underneath the event_lock because there's a queue in
the kthread that might be waiting to grab the event_lock to process.
This sounds like a good reason to rewrite event_thread to use
threaded_irq and/or workqueue.
ok, i will do
1) protect suspended flag with event_lock to prevent new event be added
2) disable interrupts
2) wait for event_q empty before turn off power
Re: [PATCH libdrm] xf86drmMode: Create drmModeCreatePropertyBlobWithFlags
Note, the headers in include/drm/ must be updated in a special manner from the kernel. See the README in the subdir for details.
[PATCH libdrm] xf86drmMode: Create drmModeCreatePropertyBlobWithFlags
[Why]
The kernel has support for creating a blob with flags, particularly
write only flag.
The user space should use libdrm library to make use of the blob flags.
[How]
Create drmModeCreatePropertyBlobWithFlags which has the same
implementation as the existing drmModeCreatePropertyBlob but with a flag
argument.
Signed-off-by: Mark Yacoub
---
include/drm/drm_mode.h | 6 ++
xf86drmMode.c | 7 +++
xf86drmMode.h | 3 +++
3 files changed, 16 insertions(+)
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 9b6722d4..b0df381f 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -991,6 +991,9 @@ struct drm_format_modifier {
__u64 modifier;
};
+#define DRM_MODE_CREATE_BLOB_WRITE_ONLY
\
+ (1 << 0) /* data of the blob can't be read by user space */
+
/**
* struct drm_mode_create_blob - Create New blob property
*
@@ -1004,6 +1007,9 @@ struct drm_mode_create_blob {
__u32 length;
/** @blob_id: Return: new property ID. */
__u32 blob_id;
+ /** Flags for special handling. */
+ __u32 flags;
+ __u32 pad;
};
/**
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 87e96603..072b395c 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -1592,6 +1592,12 @@ drm_public int
drmModeCreatePropertyBlob(int fd, const void *data, size_t length,
uint32_t *id)
{
+ return drmModeCreatePropertyBlobWithFlags(fd, data, length, id, 0);
+}
+
+extern int drmModeCreatePropertyBlobWithFlags(int fd, const void *data,
+ size_t length, uint32_t *id,
+ uint32_t flags) {
struct drm_mode_create_blob create;
int ret;
@@ -1603,6 +1609,7 @@ drmModeCreatePropertyBlob(int fd, const void *data,
size_t length,
create.length = length;
create.data = (uintptr_t) data;
create.blob_id = 0;
+ create.flags = flags;
*id = 0;
ret = DRM_IOCTL(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, );
diff --git a/xf86drmMode.h b/xf86drmMode.h
index 19bf91dd..f22b8174 100644
--- a/xf86drmMode.h
+++ b/xf86drmMode.h
@@ -450,6 +450,9 @@ extern int drmModeAtomicCommit(int fd,
extern int drmModeCreatePropertyBlob(int fd, const void *data, size_t size,
uint32_t *id);
+extern int drmModeCreatePropertyBlobWithFlags(int fd, const void *data,
+ size_t size, uint32_t *id,
+ uint32_t flags);
extern int drmModeDestroyPropertyBlob(int fd, uint32_t id);
/*
--
2.36.0.512.ge40c2bad7a-goog
Re: [PATCH] drm/meson: Fix refcount leak in meson_encoder_hdmi_init
Hi,
On 12/05/2022 14:38, Martin Blumenstingl wrote:
On Wed, May 11, 2022 at 7:41 AM Miaoqian Lin wrote:
of_find_device_by_node() takes reference, we should use put_device()
to release it when not need anymore.
Add missing put_device() in error path to avoid refcount
leak.
Fixes: 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
DRM_BRIDGE_ATTACH_NO_CONNECTOR")
Signed-off-by: Miaoqian Lin
Reviewed-by: Martin Blumenstingl
Thanks for sending this patch!
Neil, while reviewing this I noticed that on module unload we're also
not calling put_device().
This note doesn't affect this patch - but I am wondering if we need to
put that put_device() during module unload on our TODO-list?
Indeed, it should be fixed.
Neil
Best regards,
Martin
Re: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats
Thanks Christian for the comments!! On 5/11/2022 12:33 PM, Christian König wrote: > >> The single number approach, generated by atomic, wouldn't break the >> uapi, but that number won't give any meaningful information especially >> when this is targeted just for debug purpose. And just 'inode' is not >> usable for already stated reasons. > > Well, why do you want to use the ino in the first place? This is an > anonymous inode not associated with any filesystem, so that number is > meaningless anyway. > It is just for ease of debugging. Nothing more. I can quickly traverse the /sys/kernel/dmabuf/buffers/* and get complete information about the dmabuf buffers while relating to which process this buffer is allocated by, using this inode as the 'unique' reference. https://cs.android.com/android/platform/superproject/+/master:system/memory/libmeminfo/libdmabufinfo/tools/dmabuf_dump.cpp >> How about using the atomic number generated it self used as inode >> number? I see tmpfs also maintains its own inode numbers for the same >> overflow reasons[2]. > > Yeah, that could potentially work as well. > Thanks. Will work on the next version of this patch. > Regards, > Christian.
Re: [PATCH] drm: Create support for Write-Only property blob
friendly ping :)
On Tue, May 10, 2022 at 3:08 PM Mark Yacoub wrote:
>
> [Why]
> User space might need to inject data into the kernel without allowing it
> to be read again by any user space.
> An example of where this is particularly useful is secret keys fetched
> by user space and injected into the kernel to enable content protection.
>
> [How]
> Create a DRM_MODE_CREATE_BLOB_WRITE_ONLY flag used by user space to
> create a blob and mark the blob as write only.
> On reading back the blob, data will be not be copied if it's a write
> only blob
>
> Signed-off-by: Mark Yacoub
>
> ---
> drivers/gpu/drm/drm_property.c | 3 ++-
> include/drm/drm_property.h | 2 ++
> include/uapi/drm/drm_mode.h| 6 ++
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
> index dfec479830e4..afedf7109d00 100644
> --- a/drivers/gpu/drm/drm_property.c
> +++ b/drivers/gpu/drm/drm_property.c
> @@ -765,7 +765,7 @@ int drm_mode_getblob_ioctl(struct drm_device *dev,
> if (!blob)
> return -ENOENT;
>
> - if (out_resp->length == blob->length) {
> + if (out_resp->length == blob->length && !blob->is_write_only) {
> if (copy_to_user(u64_to_user_ptr(out_resp->data),
> blob->data,
> blob->length)) {
> @@ -800,6 +800,7 @@ int drm_mode_createblob_ioctl(struct drm_device *dev,
> ret = -EFAULT;
> goto out_blob;
> }
> + blob->is_write_only = out_resp->flags &
> DRM_MODE_CREATE_BLOB_WRITE_ONLY;
>
> /* Dropping the lock between create_blob and our access here is safe
> * as only the same file_priv can remove the blob; at this point, it
> is
> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> index 65bc9710a470..700782f021b9 100644
> --- a/include/drm/drm_property.h
> +++ b/include/drm/drm_property.h
> @@ -205,6 +205,7 @@ struct drm_property {
> * _mode_config.property_blob_list.
> * @head_file: entry on the per-file blob list in _file.blobs list.
> * @length: size of the blob in bytes, invariant over the lifetime of the
> object
> + * @is_write_only: user space can't read the blob data.
> * @data: actual data, embedded at the end of this structure
> *
> * Blobs are used to store bigger values than what fits directly into the 64
> @@ -219,6 +220,7 @@ struct drm_property_blob {
> struct list_head head_global;
> struct list_head head_file;
> size_t length;
> + bool is_write_only;
> void *data;
> };
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 0a0d56a6158e..de192d3813e9 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -1107,6 +1107,9 @@ struct drm_format_modifier {
> __u64 modifier;
> };
>
> +#define DRM_MODE_CREATE_BLOB_WRITE_ONLY
> \
> + (1 << 0) /* data of the blob can't be read by user space */
> +
> /**
> * struct drm_mode_create_blob - Create New blob property
> *
> @@ -1120,6 +1123,9 @@ struct drm_mode_create_blob {
> __u32 length;
> /** @blob_id: Return: new property ID. */
> __u32 blob_id;
> + /** Flags for special handling. */
> + __u32 flags;
> + __u32 pad;
> };
>
> /**
> --
> 2.36.0.512.ge40c2bad7a-goog
>
Re: [PATCH] drm: Add a debug message when getting a prop is missing
friendly ping :)
On Tue, May 10, 2022 at 2:28 PM Mark Yacoub wrote:
>
> [Why]
> If a connector property is attached but
> drm_atomic_connector_get_property doesn't handle a case for it,
> modeteset will crash with a segfault without.
>
> [How]
> Add a debug message indicating that a connector property is not handled
> when user space is trying to read it.
>
> TEST=modetest
>
> Signed-off-by: Mark Yacoub
> ---
> drivers/gpu/drm/drm_atomic_uapi.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index acb1ee93d206..36b0f664dd80 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -884,6 +884,12 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> } else {
> + // LOG that the kernel is missing handling this property as a
> case here.
> + drm_dbg_atomic(
> + dev,
> + "[CONNECTOR:%d:%s] Get Property [PROP:%d:%s] is not
> handled\n",
> + connector->base.id, connector->name,
> property->base.id,
> + property->name);
> return -EINVAL;
> }
>
> --
> 2.36.0.512.ge40c2bad7a-goog
>
Re: [PATCH v4 10/15] drm/shmem-helper: Take reservation lock instead of drm_gem_shmem locks
On Thu, May 12, 2022 at 09:29:35AM +0200, Christian König wrote: > Am 11.05.22 um 21:05 schrieb Daniel Vetter: > > [SNIP] > > > > > It's unclear to me which driver may ever want to do the mapping under > > > > > the dma_resv_lock. But if we will ever have such a driver that will > > > > > need > > > > > to map imported buffer under dma_resv_lock, then we could always add > > > > > the > > > > > dma_buf_vmap_locked() variant of the function. In this case the > > > > > locking > > > > > rule will sound like this: > > > > > > > > > > "All dma-buf importers are responsible for holding the dma-reservation > > > > > lock around the dmabuf->ops->mmap/vmap() calls." > > > Are you okay with this rule? > > Yeah I think long-term it's where we want to be, just trying to find > > clever ways to get there. > > > > And I think Christian agrees with that? > > Yes, completely. > > A design where most DMA-buf functions are supposed to be called with the > reservation lock held is exactly what I have in mind for the long term. > > > > > > > It shouldn't be that hard to clean up. The last time I looked into > > > > > > it my > > > > > > main problem was that we didn't had any easy unit test for it. > > > > > Do we have any tests for dma-bufs at all? It's unclear to me what you > > > > > are going to test in regards to the reservation locks, could you > > > > > please > > > > > clarify? > > > > Unfortunately not really :-/ Only way really is to grab a driver which > > > > needs vmap (those are mostly display drivers) on an imported buffer, and > > > > see what happens. > > > > > > > > 2nd best is liberally sprinkling lockdep annotations all over the place > > > > and throwing it at intel ci (not sure amd ci is accessible to the > > > > public) > > > > and then hoping that's good enough. Stuff like might_lock and > > > > dma_resv_assert_held. > > > Alright > > So throwing it at intel-gfx-ci can't hurt I think, but that only covers > > i915 so doesn't really help with the bigger issue of catching all the > > drivers. > > BTW: We have now somebody working on converting the existing libdrm_amdgpu > unit tests over to igt. This sounds awesome. /me throws a happy dance Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [Freedreno] [RFC v2] drm/msm: Add initial ci/ subdirectory
On Thu, May 12, 2022 at 03:28:16PM +0200, Tomeu Vizoso wrote: > On 5/11/22 7:46 PM, Rob Clark wrote: > > On Wed, May 11, 2022 at 10:12 AM Daniel Vetter wrote: > > > > > > On Tue, 10 May 2022 at 22:26, Rob Clark wrote: > > > > > > > > On Tue, May 10, 2022 at 12:39 PM Jessica Zhang > > > > wrote: > > > > > > > > > > > > > > > > > > > > On 5/10/2022 7:13 AM, Tomeu Vizoso wrote: > > > > > > And use it to store expectations about what the drm/msm driver is > > > > > > supposed to pass in the IGT test suite. > > > > > > > > > > > > Also include a configuration file that points to the out-of-tree CI > > > > > > scripts. > > > > > > > > > > > > By storing the test expectations along the code we can make sure > > > > > > both > > > > > > stay in sync with each other, and so we can know when a code change > > > > > > breaks those expectations. > > > > > > > > > > > > This will allow all contributors to drm/msm to reuse the > > > > > > infrastructure > > > > > > already in gitlab.freedesktop.org to test the driver on several > > > > > > generations of the hardware. > > > > > > > > > > > > v2: > > > > > > - Fix names of result expectation files to match SoC > > > > > > - Don't execute tests that are going to skip on all boards > > > > > > > > > > > > Signed-off-by: Tomeu Vizoso > > > > > > --- > > > > > >Documentation/gpu/msm_automated_testing.rst | 70 + > > > > > >drivers/gpu/drm/msm/ci/gitlab-ci.yml | 11 ++ > > > > > >drivers/gpu/drm/msm/ci/msm.testlist | 148 > > > > > > ++ > > > > > >.../gpu/drm/msm/ci/msm_apq8016_results.txt| 140 > > > > > > + > > > > > >.../gpu/drm/msm/ci/msm_apq8096_results.txt| 140 > > > > > > + > > > > > >drivers/gpu/drm/msm/ci/msm_sc7180_results.txt | 141 > > > > > > + > > > > > >drivers/gpu/drm/msm/ci/msm_sdm845_results.txt | 141 > > > > > > + > > > > > >7 files changed, 791 insertions(+) > > > > > >create mode 100644 Documentation/gpu/msm_automated_testing.rst > > > > > >create mode 100644 drivers/gpu/drm/msm/ci/gitlab-ci.yml > > > > > >create mode 100644 drivers/gpu/drm/msm/ci/msm.testlist > > > > > >create mode 100644 drivers/gpu/drm/msm/ci/msm_apq8016_results.txt > > > > > >create mode 100644 drivers/gpu/drm/msm/ci/msm_apq8096_results.txt > > > > > >create mode 100644 drivers/gpu/drm/msm/ci/msm_sc7180_results.txt > > > > > >create mode 100644 drivers/gpu/drm/msm/ci/msm_sdm845_results.txt > > > > > > > > > > [snip] > > > > > > > > diff --git a/drivers/gpu/drm/msm/ci/msm_sc7180_results.txt > > > > > > b/drivers/gpu/drm/msm/ci/msm_sc7180_results.txt > > > > > > new file mode 100644 > > > > > > index ..01f7b4b399b5 > > > > > > --- /dev/null > > > > > > +++ b/drivers/gpu/drm/msm/ci/msm_sc7180_results.txt > > > > > > @@ -0,0 +1,141 @@ > > > > > > +igt@core_auth@getclient-simple,dmesg-warn > > > > > > +igt@core_auth@getclient-master-drop,pass > > > > > > +igt@core_auth@basic-auth,pass > > > > > > +igt@core_auth@many-magics,pass > > > > > > +igt@core_getclient,pass > > > > > > +igt@core_getstats,pass > > > > > > +igt@core_getversion,pass > > > > > > +igt@core_setmaster_vs_auth,pass > > > > > > +igt@drm_read@invalid-buffer,pass > > > > > > +igt@drm_read@fault-buffer,pass > > > > > > +igt@drm_read@empty-block,pass > > > > > > +igt@drm_read@empty-nonblock,pass > > > > > > +igt@drm_read@short-buffer-block,pass > > > > > > +igt@drm_read@short-buffer-nonblock,pass > > > > > > +igt@drm_read@short-buffer-wakeup,pass > > > > > > +igt@kms_addfb_basic@unused-handle,pass > > > > > > +igt@kms_addfb_basic@unused-pitches,pass > > > > > > +igt@kms_addfb_basic@unused-offsets,pass > > > > > > +igt@kms_addfb_basic@unused-modifier,pass > > > > > > +igt@kms_addfb_basic@legacy-format,dmesg-warn > > > > > > +igt@kms_addfb_basic@no-handle,pass > > > > > > +igt@kms_addfb_basic@basic,pass > > > > > > +igt@kms_addfb_basic@bad-pitch-0,pass > > > > > > +igt@kms_addfb_basic@bad-pitch-32,pass > > > > > > +igt@kms_addfb_basic@bad-pitch-63,pass > > > > > > +igt@kms_addfb_basic@bad-pitch-128,pass > > > > > > +igt@kms_addfb_basic@bad-pitch-256,pass > > > > > > +igt@kms_addfb_basic@bad-pitch-1024,pass > > > > > > +igt@kms_addfb_basic@bad-pitch-999,pass > > > > > > +igt@kms_addfb_basic@bad-pitch-65536,pass > > > > > > +igt@kms_addfb_basic@size-max,pass > > > > > > +igt@kms_addfb_basic@too-wide,pass > > > > > > +igt@kms_addfb_basic@too-high,dmesg-warn > > > > > > > > > > For test results on Trogdor, is is possible to have them be > > > > > success/fail/skip only? > > > > > > > > > > Results such as dmesg-warn/dmesg-fail are igt_runner specific and > > > > > because there isn't support for igt_runner on ChromeOS, they will be > > > > > difficult to replicate and debug. > > > > > > > > Actually, I wonder if it would be better to just treat > > > > dmesg-warn/dmesg-fail as pass/fail? I'd noticed some flakes on
Re: [PATCH] drm/msm/a6xx: Fix refcount leak in a6xx_gpu_init
On 5/12/2022 5:49 PM, Miaoqian Lin wrote:
of_parse_phandle() returns a node pointer with refcount
incremented, we should use of_node_put() on it when not need anymore.
a6xx_gmu_init() passes the node to of_find_device_by_node()
and of_dma_configure(), of_find_device_by_node() will takes its
reference, of_dma_configure() doesn't need the node after usage.
Add missing of_node_put() to avoid refcount leak.
Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
Signed-off-by: Miaoqian Lin
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ccc4fcf7a630..a8f6d73197b1 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1919,6 +1919,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
BUG_ON(!node);
ret = a6xx_gmu_init(a6xx_gpu, node);
+ of_node_put(node);
if (ret) {
a6xx_destroy(&(a6xx_gpu->base.base));
return ERR_PTR(ret);
Reviewed-by: Akhil P Oommen
-Akhil.
Re: [REPORT] syscall reboot + umh + firmware fallback
On Thu, May 12, 2022 at 08:18:24PM +0900, Byungchul Park wrote: > I have a question about this one. Yes, it would never been stuck thanks > to timeout. However, IIUC, timeouts are not supposed to expire in normal > cases. So I thought a timeout expiration means not a normal case so need > to inform it in terms of dependency so as to prevent further expiraton. > That's why I have been trying to track even timeout'ed APIs. As I beleive I've already pointed out to you previously in ext4 and ocfs2, the jbd2 timeout every five seconds happens **all** the time while the file system is mounted. Commits more frequently than five seconds is the exception case, at least for desktops/laptop workloads. We *don't* get to the timeout only when a userspace process calls fsync(2), or if the journal was incorrectly sized by the system administrator so that it's too small, and the workload has so many file system mutations that we have to prematurely close the transaction ahead of the 5 second timeout. > Do you think DEPT shouldn't track timeout APIs? If I was wrong, I > shouldn't track the timeout APIs any more. DEPT tracking timeouts will cause false positives in at least some cases. At the very least, there needs to be an easy way to suppress these false positives on a per wait/mutex/spinlock basis. - Ted
[LINUX PATCH 2/2] drm: xlnx: dsi: driver for Xilinx DSI Tx subsystem
The Xilinx MIPI DSI Tx Subsystem soft IP is used to display video data from AXI-4 stream interface. It supports upto 4 lanes, optional register interface for the DPHY and multiple RGB color formats. This is a MIPI-DSI host driver and provides DSI bus for panels. This driver also helps to communicate with its panel using panel framework. Signed-off-by: Venkateshwar Rao Gannavarapu --- drivers/gpu/drm/xlnx/Kconfig| 14 ++ drivers/gpu/drm/xlnx/Makefile | 1 + drivers/gpu/drm/xlnx/xlnx_dsi.c | 456 3 files changed, 471 insertions(+) create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig index c3d0826..caa632b 100644 --- a/drivers/gpu/drm/xlnx/Kconfig +++ b/drivers/gpu/drm/xlnx/Kconfig @@ -14,3 +14,17 @@ config DRM_ZYNQMP_DPSUB This is a DRM/KMS driver for ZynqMP DisplayPort controller. Choose this option if you have a Xilinx ZynqMP SoC with DisplayPort subsystem. + +config DRM_XLNX_DSI + tristate "Xilinx DRM DSI Subsystem Driver" + depends on DRM && OF + select DRM_KMS_HELPER + select DRM_MIPI_DSI + select DRM_PANEL + select BACKLIGHT_LCD_SUPPORT + select BACKLIGHT_CLASS_DEVICE + select DRM_PANEL_SIMPLE + help + DRM bridge driver for Xilinx programmable DSI subsystem controller. + choose this option if you hava a Xilinx MIPI-DSI Tx subsytem in + video pipeline. diff --git a/drivers/gpu/drm/xlnx/Makefile b/drivers/gpu/drm/xlnx/Makefile index 51c24b7..1e97fbe 100644 --- a/drivers/gpu/drm/xlnx/Makefile +++ b/drivers/gpu/drm/xlnx/Makefile @@ -1,2 +1,3 @@ zynqmp-dpsub-y := zynqmp_disp.o zynqmp_dpsub.o zynqmp_dp.o obj-$(CONFIG_DRM_ZYNQMP_DPSUB) += zynqmp-dpsub.o +obj-$(CONFIG_DRM_XLNX_DSI) += xlnx_dsi.o diff --git a/drivers/gpu/drm/xlnx/xlnx_dsi.c b/drivers/gpu/drm/xlnx/xlnx_dsi.c new file mode 100644 index 000..a5291f3 --- /dev/null +++ b/drivers/gpu/drm/xlnx/xlnx_dsi.c @@ -0,0 +1,456 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * + * Xilinx FPGA MIPI DSI Tx Controller driver. + * + * Copyright (C) 2022 Xilinx, Inc. + * + * Author: Venkateshwar Rao G + * + */ + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +/* DSI Tx IP registers */ +#define XDSI_CCR 0x00 +#define XDSI_CCR_COREENB BIT(0) +#define XDSI_CCR_SOFTRST BIT(1) +#define XDSI_CCR_CRREADY BIT(2) +#define XDSI_CCR_CMDMODE BIT(3) +#define XDSI_CCR_DFIFORST BIT(4) +#define XDSI_CCR_CMDFIFORSTBIT(5) +#define XDSI_PCR 0x04 +#define XDSI_PCR_LANES_MASK3 +#define XDSI_PCR_VIDEOMODE(x) (((x) & 0x3) << 3) +#define XDSI_PCR_VIDEOMODE_MASK(0x3 << 3) +#define XDSI_PCR_VIDEOMODE_SHIFT 3 +#define XDSI_PCR_BLLPTYPE(x) ((x) << 5) +#define XDSI_PCR_BLLPMODE(x) ((x) << 6) +#define XDSI_PCR_PIXELFORMAT_MASK (0x3 << 11) +#define XDSI_PCR_PIXELFORMAT_SHIFT 11 +#define XDSI_PCR_EOTPENABLE(x) ((x) << 13) +#define XDSI_GIER 0x20 +#define XDSI_ISR 0x24 +#define XDSI_IER 0x28 +#define XDSI_STR 0x2C +#define XDSI_STR_RDY_SHPKT BIT(6) +#define XDSI_STR_RDY_LNGPKTBIT(7) +#define XDSI_STR_DFIFO_FULLBIT(8) +#define XDSI_STR_DFIFO_EMPTY BIT(9) +#define XDSI_STR_WAITFR_DATA BIT(10) +#define XDSI_STR_CMD_EXE_PGS BIT(11) +#define XDSI_STR_CCMD_PROC BIT(12) +#define XDSI_STR_LPKT_MASK (0x5 << 7) +#define XDSI_CMD 0x30 +#define XDSI_CMD_QUEUE_PACKET(x) ((x) & GENMASK(23, 0)) +#define XDSI_DFR 0x34 +#define XDSI_TIME1 0x50 +#define XDSI_TIME1_BLLP_BURST(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME1_HSA(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_TIME2 0x54 +#define XDSI_TIME2_VACT(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME2_HACT(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_HACT_MULTIPLIER GENMASK(1, 0) +#define XDSI_TIME3 0x58 +#define XDSI_TIME3_HFP(x) ((x) & GENMASK(15, 0)) +#define XDSI_TIME3_HBP(x) (((x) & GENMASK(15, 0)) << 16) +#define XDSI_TIME4 0x5c +#define XDSI_TIME4_VFP(x) ((x) & GENMASK(7, 0)) +#define XDSI_TIME4_VBP(x) (((x) & GENMASK(7, 0)) << 8) +#define XDSI_TIME4_VSA(x) (((x) & GENMASK(7, 0)) << 16) +#define XDSI_NUM_DATA_T4 + +/** + * struct xlnx_dsi - Xilinx DSI-TX core + * @bridge: DRM bridge structure + * @dsi_host: DSI host device + * @panel_bridge: Panel bridge structure + * @panel: DRM panel structure + * @dev:
[LINUX PATCH 0/2] Add Xilinx DSI-Tx DRM driver
MIPI DSI TX subsystem allows you to quickly create systems based on the MIPI protocol. It interfaces between the video processing subsystems and MIPI-based displays. An internal high-speed physical layer design, D-PHY, is provided to allow direct connection to display peripherals. The subsystem consists of the following sub-blocks: - MIPI D-PHY - MIPI DSI TX Controller - AXI Crossbar Please refer pg238 [1]. The DSI TX Controller receives stream of image data through an input stream interface. At design time, this subsystem can be configured to number of lanes and pixel format. This patch series adds the dt-binding and DRM driver for Xilinx DSI-Tx soft IP. References: [1]: https://www.xilinx.com/support/documentation/ip_documentation/mipi_dsi_tx_subsystem/v2_0/pg238-mipi-dsi-tx.pdf Venkateshwar Rao Gannavarapu (2): dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation drm: xlnx: dsi: driver for Xilinx DSI Tx subsystem .../bindings/display/xlnx/xlnx,dsi-tx.yaml | 105 + drivers/gpu/drm/xlnx/Kconfig | 14 + drivers/gpu/drm/xlnx/Makefile | 1 + drivers/gpu/drm/xlnx/xlnx_dsi.c| 456 + 4 files changed, 576 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml create mode 100644 drivers/gpu/drm/xlnx/xlnx_dsi.c -- 1.8.3.1
[LINUX PATCH 1/2] dt-bindings: display: xlnx: Add DSI 2.0 Tx subsystem documentation
This patch adds dt binding for Xilinx DSI TX subsystem.
The Xilinx MIPI DSI (Display serial interface) Transmitter Subsystem
implements the Mobile Industry Processor Interface (MIPI) based display
interface. It supports the interface with the programmable logic (FPGA).
Signed-off-by: Venkateshwar Rao Gannavarapu
---
.../bindings/display/xlnx/xlnx,dsi-tx.yaml | 105 +
1 file changed, 105 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
new file mode 100644
index 000..8e23cf5
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,dsi-tx.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/xlnx/xlnx,dsi-tx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx DSI Transmitter subsystem
+
+maintainers:
+ - Venkateshwar Rao Gannavarapu
+
+description: |
+ The Xilinx DSI Transmitter Subsystem implements the Mobile Industry
+ Processor Interface based display interface. It supports the interface
+ with the programmable logic (FPGA).
+
+ For more details refer to PG238 Xilinx MIPI DSI-V2.0 Tx Subsystem.
+
+properties:
+ compatible:
+const: xlnx,dsi-tx-v2.0
+
+ reg:
+maxItems: 1
+
+ clocks:
+description: List of clock specifiers
+items:
+ - description: AXI Lite CPU clock
+ - description: D-phy clock
+
+ clock-names:
+items:
+ - const: s_axis_aclk
+ - const: dphy_clk_200M
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/$defs/port-base
+description:
+ Input port node to receive pixel data from the
+ display controller. Exactly one endpoint must be
+ specified.
+properties:
+ endpoint:
+$ref: /schemas/graph.yaml#/properties/endpoint
+description: sub-node describing the input from CRTC
+
+ port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+ DSI output port node to the panel or the next bridge
+ in the chain
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+dsi_tx@8002 {
+compatible = "xlnx,dsi-tx-v2.0";
+reg = <0x8002 0x2>;
+clock-names = "s_axi_aclk", "dphy_clk_200M";
+clocks = <_clk_0>, <_clk_1>;
+
+panel@0 {
+compatible = "auo,b101uan01";
+reg = <0>;
+port {
+panel_in: endpoint {
+remote-endpoint = <_dsi_out>;
+};
+};
+};
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+#size-cells = <0>;
+#address-cells = <1>;
+reg = <0>;
+mipi_dsi_in: endpoint@0 {
+reg = <0>;
+remote-endpoint = <_disp_crtc>;
+};
+};
+port@1 {
+reg = <1>;
+mipi_dsi_out: endpoint {
+remote-endpoint = <_in>;
+};
+};
+};
+};
--
1.8.3.1
Re: [BUG] Warning and NULL-ptr dereference in amdgpu driver with 5.18
On Thu, May 12, 2022 at 4:35 AM Jörg Rödel wrote: > > On Tue, May 10, 2022 at 04:41:57PM -0400, Alex Deucher wrote: > > Does setting amdgpu.runpm=0 on the kernel command line in grub help? > > If so, that should fixed with: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f95af4a9236695caed24fe6401256bb974e8f2a7 > > Unfortunatly, no, this option doesn't help. Tested with v5.18-rc6, full > dmesg attached. > > Any idea what the BadTLP messages migh be caused by? Are those new? Maybe the card is not seated correctly? Can you try another slot? As for the null pointer defer in the display code, @Wentland, Harry any ideas? I don't see why that should happen. Maybe some hotplug pin is faulty or the display has input detection and that is causing some sort of hotplug interrupt that causes a race somewhere in the driver? Can you make sure the monitor connector is firmly seated on the GPU? Alex > > Regards, > > Joerg > > -- > Jörg Rödel > jroe...@suse.de > > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5 > 90409 Nürnberg > Germany > > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev >
Re: [PATCH , v4] media: mediatek: vcodec: Fix v4l2 compliance decoder cmd test fail
Il 23/04/22 09:35, Yunfei Dong ha scritto:
Will return -EINVAL using standard framework api when test stateless
decoder with cmd VIDIOC_(TRY)DECODER_CMD. Disable them to adjust v4l2
compliance test for user driver(GStreamer/Chrome) won't use decoder cmd.
Fixes: 8cdc3794b2e3 ("media: mtk-vcodec: vdec: support stateless API")
Signed-off-by: Yunfei Dong
Reviewed-by: AngeloGioacchino Del Regno
Patch "drm/amd/display/dc/gpio/gpio_service: Pass around correct dce_{version, environment} types" has been added to the 5.4-stable tree
This is a note to let you know that I've just added the patch titled
drm/amd/display/dc/gpio/gpio_service: Pass around correct dce_{version,
environment} types
to the 5.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-amd-display-dc-gpio-gpio_service-pass-around-correct-dce_-version-environment-types.patch
and it can be found in the queue-5.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 353f7f3a9dd5fd2833b6462bac89ec1654c9c3aa Mon Sep 17 00:00:00 2001
From: Lee Jones
Date: Wed, 26 May 2021 09:47:06 +0100
Subject: drm/amd/display/dc/gpio/gpio_service: Pass around correct
dce_{version, environment} types
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Lee Jones
commit 353f7f3a9dd5fd2833b6462bac89ec1654c9c3aa upstream.
Fixes the following W=1 kernel build warning(s):
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c: In function
‘dal_gpio_service_create’:
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:71:4: warning:
implicit conversion from ‘enum dce_version’ to ‘enum dce_environment’
[-Wenum-conversion]
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:77:4: warning:
implicit conversion from ‘enum dce_version’ to ‘enum dce_environment’
[-Wenum-conversion]
Cc: Harry Wentland
Cc: Leo Li
Cc: Alex Deucher
Cc: "Christian König"
Cc: David Airlie
Cc: Daniel Vetter
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Lee Jones
Signed-off-by: Alex Deucher
Cc: Nathan Chancellor
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 12 +--
drivers/gpu/drm/amd/display/include/gpio_service_interface.h |4 +--
2 files changed, 8 insertions(+), 8 deletions(-)
--- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
+++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
@@ -53,8 +53,8 @@
*/
struct gpio_service *dal_gpio_service_create(
- enum dce_version dce_version_major,
- enum dce_version dce_version_minor,
+ enum dce_version dce_version,
+ enum dce_environment dce_environment,
struct dc_context *ctx)
{
struct gpio_service *service;
@@ -67,14 +67,14 @@ struct gpio_service *dal_gpio_service_cr
return NULL;
}
- if (!dal_hw_translate_init(>translate, dce_version_major,
- dce_version_minor)) {
+ if (!dal_hw_translate_init(>translate, dce_version,
+ dce_environment)) {
BREAK_TO_DEBUGGER();
goto failure_1;
}
- if (!dal_hw_factory_init(>factory, dce_version_major,
- dce_version_minor)) {
+ if (!dal_hw_factory_init(>factory, dce_version,
+ dce_environment)) {
BREAK_TO_DEBUGGER();
goto failure_1;
}
--- a/drivers/gpu/drm/amd/display/include/gpio_service_interface.h
+++ b/drivers/gpu/drm/amd/display/include/gpio_service_interface.h
@@ -42,8 +42,8 @@ void dal_gpio_destroy(
struct gpio **ptr);
struct gpio_service *dal_gpio_service_create(
- enum dce_version dce_version_major,
- enum dce_version dce_version_minor,
+ enum dce_version dce_version,
+ enum dce_environment dce_environment,
struct dc_context *ctx);
struct gpio *dal_gpio_service_create_irq(
Patches currently in stable-queue which might be from lee.jo...@linaro.org are
queue-5.4/drm-amd-display-dc-gpio-gpio_service-pass-around-correct-dce_-version-environment-types.patch
queue-5.4/block-drbd-drbd_nl-make-conversion-to-enum-drbd_ret_code-explicit.patch
Patch "drm/amd/display/dc/gpio/gpio_service: Pass around correct dce_{version, environment} types" has been added to the 4.19-stable tree
This is a note to let you know that I've just added the patch titled
drm/amd/display/dc/gpio/gpio_service: Pass around correct dce_{version,
environment} types
to the 4.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-amd-display-dc-gpio-gpio_service-pass-around-correct-dce_-version-environment-types.patch
and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 353f7f3a9dd5fd2833b6462bac89ec1654c9c3aa Mon Sep 17 00:00:00 2001
From: Lee Jones
Date: Wed, 26 May 2021 09:47:06 +0100
Subject: drm/amd/display/dc/gpio/gpio_service: Pass around correct
dce_{version, environment} types
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Lee Jones
commit 353f7f3a9dd5fd2833b6462bac89ec1654c9c3aa upstream.
Fixes the following W=1 kernel build warning(s):
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c: In function
‘dal_gpio_service_create’:
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:71:4: warning:
implicit conversion from ‘enum dce_version’ to ‘enum dce_environment’
[-Wenum-conversion]
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:77:4: warning:
implicit conversion from ‘enum dce_version’ to ‘enum dce_environment’
[-Wenum-conversion]
Cc: Harry Wentland
Cc: Leo Li
Cc: Alex Deucher
Cc: "Christian König"
Cc: David Airlie
Cc: Daniel Vetter
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Lee Jones
Signed-off-by: Alex Deucher
Cc: Nathan Chancellor
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 12 +--
drivers/gpu/drm/amd/display/include/gpio_service_interface.h |4 +--
2 files changed, 8 insertions(+), 8 deletions(-)
--- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
+++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
@@ -51,8 +51,8 @@
*/
struct gpio_service *dal_gpio_service_create(
- enum dce_version dce_version_major,
- enum dce_version dce_version_minor,
+ enum dce_version dce_version,
+ enum dce_environment dce_environment,
struct dc_context *ctx)
{
struct gpio_service *service;
@@ -66,14 +66,14 @@ struct gpio_service *dal_gpio_service_cr
return NULL;
}
- if (!dal_hw_translate_init(>translate, dce_version_major,
- dce_version_minor)) {
+ if (!dal_hw_translate_init(>translate, dce_version,
+ dce_environment)) {
BREAK_TO_DEBUGGER();
goto failure_1;
}
- if (!dal_hw_factory_init(>factory, dce_version_major,
- dce_version_minor)) {
+ if (!dal_hw_factory_init(>factory, dce_version,
+ dce_environment)) {
BREAK_TO_DEBUGGER();
goto failure_1;
}
--- a/drivers/gpu/drm/amd/display/include/gpio_service_interface.h
+++ b/drivers/gpu/drm/amd/display/include/gpio_service_interface.h
@@ -42,8 +42,8 @@ void dal_gpio_destroy(
struct gpio **ptr);
struct gpio_service *dal_gpio_service_create(
- enum dce_version dce_version_major,
- enum dce_version dce_version_minor,
+ enum dce_version dce_version,
+ enum dce_environment dce_environment,
struct dc_context *ctx);
struct gpio *dal_gpio_service_create_irq(
Patches currently in stable-queue which might be from lee.jo...@linaro.org are
queue-4.19/drm-amd-display-dc-gpio-gpio_service-pass-around-correct-dce_-version-environment-types.patch
queue-4.19/block-drbd-drbd_nl-make-conversion-to-enum-drbd_ret_code-explicit.patch
Patch "drm/amd/display/dc/gpio/gpio_service: Pass around correct dce_{version, environment} types" has been added to the 5.10-stable tree
This is a note to let you know that I've just added the patch titled
drm/amd/display/dc/gpio/gpio_service: Pass around correct dce_{version,
environment} types
to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drm-amd-display-dc-gpio-gpio_service-pass-around-correct-dce_-version-environment-types.patch
and it can be found in the queue-5.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
>From 353f7f3a9dd5fd2833b6462bac89ec1654c9c3aa Mon Sep 17 00:00:00 2001
From: Lee Jones
Date: Wed, 26 May 2021 09:47:06 +0100
Subject: drm/amd/display/dc/gpio/gpio_service: Pass around correct
dce_{version, environment} types
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Lee Jones
commit 353f7f3a9dd5fd2833b6462bac89ec1654c9c3aa upstream.
Fixes the following W=1 kernel build warning(s):
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c: In function
‘dal_gpio_service_create’:
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:71:4: warning:
implicit conversion from ‘enum dce_version’ to ‘enum dce_environment’
[-Wenum-conversion]
drivers/gpu/drm/amd/amdgpu/../display/dc/gpio/gpio_service.c:77:4: warning:
implicit conversion from ‘enum dce_version’ to ‘enum dce_environment’
[-Wenum-conversion]
Cc: Harry Wentland
Cc: Leo Li
Cc: Alex Deucher
Cc: "Christian König"
Cc: David Airlie
Cc: Daniel Vetter
Cc: amd-...@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Lee Jones
Signed-off-by: Alex Deucher
Cc: Nathan Chancellor
Signed-off-by: Greg Kroah-Hartman
---
drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c | 12 +--
drivers/gpu/drm/amd/display/include/gpio_service_interface.h |4 +--
2 files changed, 8 insertions(+), 8 deletions(-)
--- a/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
+++ b/drivers/gpu/drm/amd/display/dc/gpio/gpio_service.c
@@ -53,8 +53,8 @@
*/
struct gpio_service *dal_gpio_service_create(
- enum dce_version dce_version_major,
- enum dce_version dce_version_minor,
+ enum dce_version dce_version,
+ enum dce_environment dce_environment,
struct dc_context *ctx)
{
struct gpio_service *service;
@@ -67,14 +67,14 @@ struct gpio_service *dal_gpio_service_cr
return NULL;
}
- if (!dal_hw_translate_init(>translate, dce_version_major,
- dce_version_minor)) {
+ if (!dal_hw_translate_init(>translate, dce_version,
+ dce_environment)) {
BREAK_TO_DEBUGGER();
goto failure_1;
}
- if (!dal_hw_factory_init(>factory, dce_version_major,
- dce_version_minor)) {
+ if (!dal_hw_factory_init(>factory, dce_version,
+ dce_environment)) {
BREAK_TO_DEBUGGER();
goto failure_1;
}
--- a/drivers/gpu/drm/amd/display/include/gpio_service_interface.h
+++ b/drivers/gpu/drm/amd/display/include/gpio_service_interface.h
@@ -42,8 +42,8 @@ void dal_gpio_destroy(
struct gpio **ptr);
struct gpio_service *dal_gpio_service_create(
- enum dce_version dce_version_major,
- enum dce_version dce_version_minor,
+ enum dce_version dce_version,
+ enum dce_environment dce_environment,
struct dc_context *ctx);
struct gpio *dal_gpio_service_create_irq(
Patches currently in stable-queue which might be from lee.jo...@linaro.org are
queue-5.10/drm-amd-display-dc-gpio-gpio_service-pass-around-correct-dce_-version-environment-types.patch
queue-5.10/block-drbd-drbd_nl-make-conversion-to-enum-drbd_ret_code-explicit.patch
Re: [Freedreno] [RFC v2] drm/msm: Add initial ci/ subdirectory
On 5/11/22 7:46 PM, Rob Clark wrote: On Wed, May 11, 2022 at 10:12 AM Daniel Vetter wrote: On Tue, 10 May 2022 at 22:26, Rob Clark wrote: On Tue, May 10, 2022 at 12:39 PM Jessica Zhang wrote: On 5/10/2022 7:13 AM, Tomeu Vizoso wrote: And use it to store expectations about what the drm/msm driver is supposed to pass in the IGT test suite. Also include a configuration file that points to the out-of-tree CI scripts. By storing the test expectations along the code we can make sure both stay in sync with each other, and so we can know when a code change breaks those expectations. This will allow all contributors to drm/msm to reuse the infrastructure already in gitlab.freedesktop.org to test the driver on several generations of the hardware. v2: - Fix names of result expectation files to match SoC - Don't execute tests that are going to skip on all boards Signed-off-by: Tomeu Vizoso --- Documentation/gpu/msm_automated_testing.rst | 70 + drivers/gpu/drm/msm/ci/gitlab-ci.yml | 11 ++ drivers/gpu/drm/msm/ci/msm.testlist | 148 ++ .../gpu/drm/msm/ci/msm_apq8016_results.txt| 140 + .../gpu/drm/msm/ci/msm_apq8096_results.txt| 140 + drivers/gpu/drm/msm/ci/msm_sc7180_results.txt | 141 + drivers/gpu/drm/msm/ci/msm_sdm845_results.txt | 141 + 7 files changed, 791 insertions(+) create mode 100644 Documentation/gpu/msm_automated_testing.rst create mode 100644 drivers/gpu/drm/msm/ci/gitlab-ci.yml create mode 100644 drivers/gpu/drm/msm/ci/msm.testlist create mode 100644 drivers/gpu/drm/msm/ci/msm_apq8016_results.txt create mode 100644 drivers/gpu/drm/msm/ci/msm_apq8096_results.txt create mode 100644 drivers/gpu/drm/msm/ci/msm_sc7180_results.txt create mode 100644 drivers/gpu/drm/msm/ci/msm_sdm845_results.txt [snip] diff --git a/drivers/gpu/drm/msm/ci/msm_sc7180_results.txt b/drivers/gpu/drm/msm/ci/msm_sc7180_results.txt new file mode 100644 index ..01f7b4b399b5 --- /dev/null +++ b/drivers/gpu/drm/msm/ci/msm_sc7180_results.txt @@ -0,0 +1,141 @@ +igt@core_auth@getclient-simple,dmesg-warn +igt@core_auth@getclient-master-drop,pass +igt@core_auth@basic-auth,pass +igt@core_auth@many-magics,pass +igt@core_getclient,pass +igt@core_getstats,pass +igt@core_getversion,pass +igt@core_setmaster_vs_auth,pass +igt@drm_read@invalid-buffer,pass +igt@drm_read@fault-buffer,pass +igt@drm_read@empty-block,pass +igt@drm_read@empty-nonblock,pass +igt@drm_read@short-buffer-block,pass +igt@drm_read@short-buffer-nonblock,pass +igt@drm_read@short-buffer-wakeup,pass +igt@kms_addfb_basic@unused-handle,pass +igt@kms_addfb_basic@unused-pitches,pass +igt@kms_addfb_basic@unused-offsets,pass +igt@kms_addfb_basic@unused-modifier,pass +igt@kms_addfb_basic@legacy-format,dmesg-warn +igt@kms_addfb_basic@no-handle,pass +igt@kms_addfb_basic@basic,pass +igt@kms_addfb_basic@bad-pitch-0,pass +igt@kms_addfb_basic@bad-pitch-32,pass +igt@kms_addfb_basic@bad-pitch-63,pass +igt@kms_addfb_basic@bad-pitch-128,pass +igt@kms_addfb_basic@bad-pitch-256,pass +igt@kms_addfb_basic@bad-pitch-1024,pass +igt@kms_addfb_basic@bad-pitch-999,pass +igt@kms_addfb_basic@bad-pitch-65536,pass +igt@kms_addfb_basic@size-max,pass +igt@kms_addfb_basic@too-wide,pass +igt@kms_addfb_basic@too-high,dmesg-warn For test results on Trogdor, is is possible to have them be success/fail/skip only? Results such as dmesg-warn/dmesg-fail are igt_runner specific and because there isn't support for igt_runner on ChromeOS, they will be difficult to replicate and debug. Actually, I wonder if it would be better to just treat dmesg-warn/dmesg-fail as pass/fail? I'd noticed some flakes on rockchip which looked just like unrelated dmesg msg which just happened to show up while the test was running. This is kinda the reason behind standardizing on drm dmesg logging, so that we have some chances at filtering stuff out. Not sure that's a good idea, since when your entire box splats and lockdep is dead, then continuing to run drm tests is still fairly pointless. I'm not sure if we are using it yet for drm-ci, but for mesa-ci we monitor dmesg (over serial port, from the controller) for splats, so we already have the tech for restarting or aborting the CI run. We don't need igt-runner to tell us. Yep, these scripts are currently being used as-is from Mesa, so we got that functionality for free. I think this is another reason why trying at least to standardize this stuff over drivers would be pretty good idea. Additionally, some of the tests, like msm_recovery, are *expected* to generate some dmesg spam since they are intentionally triggering GPU hangs to test the recovery mechanism. Uh I don't like that. It just allows userspace to spam dmesg, which doesn't seem like a great idea. That's at least why i915 dumps these at a lower level, and in the past had a special "I'm going to whack the gpu real
Re: [PATCH v7 0/6] Proposal for a GPU cgroup controller
Le mercredi 11 mai 2022 à 13:31 -0700, T.J. Mercier a écrit :
> On Wed, May 11, 2022 at 6:21 AM Nicolas Dufresne wrote:
> >
> > Hi,
> >
> > Le mardi 10 mai 2022 à 23:56 +, T.J. Mercier a écrit :
> > > This patch series revisits the proposal for a GPU cgroup controller to
> > > track and limit memory allocations by various device/allocator
> > > subsystems. The patch series also contains a simple prototype to
> > > illustrate how Android intends to implement DMA-BUF allocator
> > > attribution using the GPU cgroup controller. The prototype does not
> > > include resource limit enforcements.
> >
> > I'm sorry, since I'm not in-depth technically involve. But from reading the
> > topic I don't understand the bound this creates between DMABuf Heaps and
> > GPU. Is
> > this an attempt to really track the DMABuf allocated by userland, or just
> > something for GPU ? What about V4L2 devices ? Any way this can be clarified,
> > specially what would other subsystem needs to have cgroup DMABuf allocation
> > controller support ?
> >
> Hi Nicolas,
>
> The link between dmabufs, dmabuf heaps, and "GPU memory" is maybe
> somewhat of an Androidism. However this change aims to be usable for
> tracking all GPU related allocations. It's just that this initial
> series only adds support for tracking dmabufs allocated from dmabuf
> heaps.
>
> In Android most graphics buffers are dma buffers allocated from a
> dmabuf heap, so that is why these dmabuf heap allocations are being
> tracked under the GPU cgroup. Other dmabuf exporters like V4L2 might
> also want to track their buffers, but would probably want to do so
> under a bucket name of something like "v4l2". Same goes for GEM
> dmabufs. The naming scheme for this is still yet to be decided. It
> would be cool to be able to attribute memory at the driver level, or
> even different types of memory at the driver level, but I imagine
> there is a point of diminishing returns for fine-grained
> naming/bucketing.
>
> So far, I haven't tried to create a strict definition of what is and
> is not "GPU memory" for the purpose of this accounting, so I don't
> think we should be restricted to tracking just dmabufs. I don't see
> why this couldn't be anything a driver wants to consider as GPU memory
> as long as it is named/bucketed appropriately, such as both on-package
> graphics card memory use and CPU memory dedicated for graphics use
> like for host/device transfers.
>
> Is that helpful?
I'm actually happy I've asked this question, wasn't silly after all. I think the
problem here is a naming issue. What you really are monitor is "video memory",
which consist of a memory segment allocated to store data used to render images
(its not always images of course, GPU an VPU have specialized buffers for their
purpose).
Whether this should be split between what is used specifically by the GPU
drivers, the display drivers, the VPU (CODEC and pre/post-processor) or camera
drivers is something that should be discussed. But in the current approach, you
really meant Video memory as a superset of the above. Personally, I think
generically (to de-Andronized your work), en-globing all video memory is
sufficient. What I fail to understand is how you will manage to distinguished
DMABuf Heap allocation (which are used outside of Android btw), from Video
allocation or other type of usage. I'm sure non-video usage will exist in the
future (think of machine learning, compute, other high bandwidth streaming
thingy ...)
>
> Best,
> T.J.
>
> > >
> > > Changelog:
> > > v7:
> > > Hide gpucg and gpucg_bucket struct definitions per Michal Koutný.
> > > This means gpucg_register_bucket now returns an internally allocated
> > > struct gpucg_bucket.
> > >
> > > Move all public function documentation to the cgroup_gpu.h header.
> > >
> > > Remove comment in documentation about duplicate name rejection which
> > > is not relevant to cgroups users per Michal Koutný.
> > >
> > > v6:
> > > Move documentation into cgroup-v2.rst per Tejun Heo.
> > >
> > > Rename BINDER_FD{A}_FLAG_SENDER_NO_NEED ->
> > > BINDER_FD{A}_FLAG_XFER_CHARGE per Carlos Llamas.
> > >
> > > Return error on transfer failure per Carlos Llamas.
> > >
> > > v5:
> > > Rebase on top of v5.18-rc3
> > >
> > > Drop the global GPU cgroup "total" (sum of all device totals) portion
> > > of the design since there is no currently known use for this per
> > > Tejun Heo.
> > >
> > > Fix commit message which still contained the old name for
> > > dma_buf_transfer_charge per Michal Koutný.
> > >
> > > Remove all GPU cgroup code except what's necessary to support charge
> > > transfer
> > > from dma_buf. Previously charging was done in export, but for non-Android
> > > graphics use-cases this is not ideal since there may be a delay between
> > > allocation and export, during which time there is no accounting.
> > >
> > > Merge dmabuf: Use the GPU cgroup charge/uncharge APIs patch into
> > > dmabuf: heaps: export system_heap buffers
Re: [PATCH v11 20/24] arm64: dts: rockchip: enable vop2 and hdmi tx on rock-3a
On Thu, May 12, 2022 at 8:17 AM Robin Murphy wrote: > > On 2022-05-08 17:53, Peter Geis wrote: > > On Sun, May 8, 2022 at 9:40 AM Piotr Oniszczuk > > wrote: > >> > >> > >> > >>> Wiadomość napisana przez Sascha Hauer w dniu > >>> 22.04.2022, o godz. 09:28: > >>> > >>> From: Michael Riesch > >>> > >>> Enable the RK356x Video Output Processor (VOP) 2 on the Radxa > >>> ROCK3 Model A. > >>> > >>> Signed-off-by: Michael Riesch > >>> Reported-by: kernel test robot > >>> Link: > >>> https://lore.kernel.org/r/20220310210352.451136-4-michael.rie...@wolfvision.net > >>> Signed-off-by: Sascha Hauer > >>> --- > >> > >> Sascha, Michael, > > > > Good Afternoon, > >> > >> I'm using v11 series on 5.18-rc5 on rk3566 tvbox with great success. > >> Recently i started to work on rock3-a (rk3568). > >> v11 gives me video, audio - but cec is not working on rock3-a. > >> > >> I was told: > >> > >> 32k clock needed for cec and this clock is generated by the rtc which is > >> embedded in the rk8xx regulator. > >> So you should make sure it is enabled when hdmi is powerd on, eg adding it > >> to the RK3568_PD_VO powerdomain should help > >> > >> I was trying to do this in dts https://pastebin.com/67wu9QrH but cec is > >> still non-functional > >> > >> Maybe You have some hints/pointers here? > > > > Add the following to the HDMI node: > > assigned-clocks = < CLK_HDMI_CEC>; > > assigned-clock-rates = <32768>; > > > > The issue is the clk_rtc32k_frac clock that feeds clk_rtc_32k which > > feeds clk_hdmi_cec is 24mhz at boot, which is too high for CEC to > > function. > > I submitted a patch to have the hdmi driver handle this, but it broke > > other SoCs because 32k is an optional clock. > > Since this is the case, I'd like Robin to weigh in on going the > > assigned-clock route again. > > (did you mean to CC me or have I missed another thread elsewhere?) Apologies, I made an unsafe assumption here. > > FWIW I still think it would be good to fix the clock driver(s) and/or > DTs to correctly deal with the availability and configuration of xin_32k > where appropriate. However, much like the HCLK_VO mess I guess that's a > larger cleanup tangent in its own right, so using "assigned-clocks" for > this one case in the meantime doesn't seem unreasonable. I was > optimistic for the cleanest, most generic solution, but if reality gets > in the way then oh well. I was thinking about this problem and came to a realization. The root dtsi files all have clk32k_in defined, even though it's listed as an optional clock. I think this should move to the device boards (much like the gmac input clock) that have it. The clock driver might need some help coping with it being missing, I haven't tested this. > > Judging by the datasheet, RK3568 might actually have a similar situation > with its clk32k_in pin, so you may want "assigned-clock-parents" as well > to ensure the whole clk_rtc32k branch is really set up the way you > currently expect - baking any more assumptions into DTBs now only seems > to add potential for breakage if kernel behaviour changes in future. rk3568 defaults to using a clock divider from the 24m clock, so it works even in the absence of clk32_in. It seemed rk3399 did as well, but unlike rk3568 it would switch to clk32k_in if the exact frequency was chosen. Implementing the above would fix that issue, and we can then implement the driver fix. > > Robin. Very Respectfully, Peter
Re: [PATCH] drm/meson: Fix refcount leak in meson_encoder_hdmi_init
On Wed, May 11, 2022 at 7:41 AM Miaoqian Lin wrote:
>
> of_find_device_by_node() takes reference, we should use put_device()
> to release it when not need anymore.
> Add missing put_device() in error path to avoid refcount
> leak.
>
> Fixes: 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
> DRM_BRIDGE_ATTACH_NO_CONNECTOR")
> Signed-off-by: Miaoqian Lin
Reviewed-by: Martin Blumenstingl
Thanks for sending this patch!
Neil, while reviewing this I noticed that on module unload we're also
not calling put_device().
This note doesn't affect this patch - but I am wondering if we need to
put that put_device() during module unload on our TODO-list?
Best regards,
Martin
Re: [PATCH] drm/meson: Fix refcount leak in meson_encoder_hdmi_init
On 11/05/2022 07:40, Miaoqian Lin wrote:
of_find_device_by_node() takes reference, we should use put_device()
to release it when not need anymore.
Add missing put_device() in error path to avoid refcount
leak.
Fixes: 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
DRM_BRIDGE_ATTACH_NO_CONNECTOR")
Signed-off-by: Miaoqian Lin
---
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 5e306de6f485..de87f02cd388 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -435,8 +435,10 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
cec_fill_conn_info_from_drm(_info,
meson_encoder_hdmi->connector);
notifier = cec_notifier_conn_register(>dev, NULL, _info);
- if (!notifier)
+ if (!notifier) {
+ put_device(>dev);
return -ENOMEM;
+ }
meson_encoder_hdmi->cec_notifier = notifier;
}
Reviewed-by: Neil Armstrong
[PATCH] drm/msm/a6xx: Fix refcount leak in a6xx_gpu_init
of_parse_phandle() returns a node pointer with refcount
incremented, we should use of_node_put() on it when not need anymore.
a6xx_gmu_init() passes the node to of_find_device_by_node()
and of_dma_configure(), of_find_device_by_node() will takes its
reference, of_dma_configure() doesn't need the node after usage.
Add missing of_node_put() to avoid refcount leak.
Fixes: 4b565ca5a2cb ("drm/msm: Add A6XX device support")
Signed-off-by: Miaoqian Lin
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ccc4fcf7a630..a8f6d73197b1 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1919,6 +1919,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
BUG_ON(!node);
ret = a6xx_gmu_init(a6xx_gpu, node);
+ of_node_put(node);
if (ret) {
a6xx_destroy(&(a6xx_gpu->base.base));
return ERR_PTR(ret);
--
2.25.1
Re: [PATCH v11 20/24] arm64: dts: rockchip: enable vop2 and hdmi tx on rock-3a
On 2022-05-08 17:53, Peter Geis wrote: On Sun, May 8, 2022 at 9:40 AM Piotr Oniszczuk wrote: Wiadomość napisana przez Sascha Hauer w dniu 22.04.2022, o godz. 09:28: From: Michael Riesch Enable the RK356x Video Output Processor (VOP) 2 on the Radxa ROCK3 Model A. Signed-off-by: Michael Riesch Reported-by: kernel test robot Link: https://lore.kernel.org/r/20220310210352.451136-4-michael.rie...@wolfvision.net Signed-off-by: Sascha Hauer --- Sascha, Michael, Good Afternoon, I'm using v11 series on 5.18-rc5 on rk3566 tvbox with great success. Recently i started to work on rock3-a (rk3568). v11 gives me video, audio - but cec is not working on rock3-a. I was told: 32k clock needed for cec and this clock is generated by the rtc which is embedded in the rk8xx regulator. So you should make sure it is enabled when hdmi is powerd on, eg adding it to the RK3568_PD_VO powerdomain should help I was trying to do this in dts https://pastebin.com/67wu9QrH but cec is still non-functional Maybe You have some hints/pointers here? Add the following to the HDMI node: assigned-clocks = < CLK_HDMI_CEC>; assigned-clock-rates = <32768>; The issue is the clk_rtc32k_frac clock that feeds clk_rtc_32k which feeds clk_hdmi_cec is 24mhz at boot, which is too high for CEC to function. I submitted a patch to have the hdmi driver handle this, but it broke other SoCs because 32k is an optional clock. Since this is the case, I'd like Robin to weigh in on going the assigned-clock route again. (did you mean to CC me or have I missed another thread elsewhere?) FWIW I still think it would be good to fix the clock driver(s) and/or DTs to correctly deal with the availability and configuration of xin_32k where appropriate. However, much like the HCLK_VO mess I guess that's a larger cleanup tangent in its own right, so using "assigned-clocks" for this one case in the meantime doesn't seem unreasonable. I was optimistic for the cleanest, most generic solution, but if reality gets in the way then oh well. Judging by the datasheet, RK3568 might actually have a similar situation with its clk32k_in pin, so you may want "assigned-clock-parents" as well to ensure the whole clk_rtc32k branch is really set up the way you currently expect - baking any more assumptions into DTBs now only seems to add potential for breakage if kernel behaviour changes in future. Robin.
[PATCH] video: fbdev: Fix refcount leak in clcdfb_of_vram_setup
of_parse_phandle() returns a node pointer with refcount
incremented, we should use of_node_put() on it when not need anymore.
Add missing of_node_put() to avoid refcount leak.
Fixes: d10715be03bd ("video: ARM CLCD: Add DT support")
Signed-off-by: Miaoqian Lin
---
drivers/video/fbdev/amba-clcd.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 9ec969e136bf..8080116aea84 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -758,12 +758,15 @@ static int clcdfb_of_vram_setup(struct clcd_fb *fb)
return -ENODEV;
fb->fb.screen_base = of_iomap(memory, 0);
- if (!fb->fb.screen_base)
+ if (!fb->fb.screen_base) {
+ of_node_put(memory);
return -ENOMEM;
+ }
fb->fb.fix.smem_start = of_translate_address(memory,
of_get_address(memory, 0, , NULL));
fb->fb.fix.smem_len = size;
+ of_node_put(memory);
return 0;
}
--
2.25.1
Re: [PATCH v1 0/7] drm/bridge_connector: perform HPD enablement automatically
On 29/04/2022 21:51, Dmitry Baryshkov wrote: From all the drivers using drm_bridge_connector only iMX/dcss and OMAP DRM driver do a proper work of calling drm_bridge_connector_en/disable_hpd() in right places. Rather than teaching each and every driver how to properly handle drm_bridge_connector's HPD, make that automatic. Add two additional drm_connector helper funcs: enable_hpd() and disable_hpd(). Make drm_kms_helper_poll_* functions call them (as this is the time where the drm_bridge_connector's functions are called by the drivers too). Gracious ping regarding this series. It went for two weeks w/o review. Few additional points 'pro': - It makes it possible to handle hpd enablement in cases where the driver uses a mixture of drm_bridge_connector and old connectors (msm) - It makes it possible for other connectors to also implement dynamic hpd enablement/disablement in a standard way Dmitry Baryshkov (7): drm/poll-helper: merge drm_kms_helper_poll_disable() and _fini() drm/probe-helper: enable and disable HPD on connectors drm/bridge_connector: rely on drm_kms_helper_poll_* for HPD enablement drm/imx/dcss: stop using drm_bridge_connector_en/disable_hpd() drm/msm/hdmi: stop using drm_bridge_connector_en/disable_hpd() drm/omap: stop using drm_bridge_connector_en/disable_hpd() drm/bridge_connector: drop drm_bridge_connector_en/disable_hpd() drivers/gpu/drm/drm_bridge_connector.c | 23 +++-- drivers/gpu/drm/drm_probe_helper.c | 40 ++- drivers/gpu/drm/imx/dcss/dcss-dev.c | 4 --- drivers/gpu/drm/imx/dcss/dcss-kms.c | 4 --- drivers/gpu/drm/msm/hdmi/hdmi.c | 2 -- drivers/gpu/drm/omapdrm/omap_drv.c | 41 include/drm/drm_bridge_connector.h | 2 -- include/drm/drm_modeset_helper_vtables.h | 22 + 8 files changed, 58 insertions(+), 80 deletions(-) -- With best wishes Dmitry
Re: [PATCH v2 2/2] drm/vkms: return early if compose_plane fails
On 04/15, Tales Lelo da Aparecida wrote:
> Do not exit quietly from compose_plane. If any plane has an invalid
> map, propagate the error value upwards. While here, add log messages
> for the invalid index.
>
> Signed-off-by: Tales Lelo da Aparecida
> ---
> drivers/gpu/drm/vkms/vkms_composer.c | 30 ++--
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
> b/drivers/gpu/drm/vkms/vkms_composer.c
> index b47ac170108c..c0a3b53cd155 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -149,16 +149,16 @@ static void blend(void *vaddr_dst, void *vaddr_src,
> }
> }
>
> -static void compose_plane(struct vkms_composer *primary_composer,
> - struct vkms_composer *plane_composer,
> - void *vaddr_out)
> +static int compose_plane(struct vkms_composer *primary_composer,
> + struct vkms_composer *plane_composer,
> + void *vaddr_out)
> {
> struct drm_framebuffer *fb = _composer->fb;
> void *vaddr;
> void (*pixel_blend)(const u8 *p_src, u8 *p_dst);
>
> if (WARN_ON(iosys_map_is_null(_composer->map[0])))
> - return;
> + return -EINVAL;
>
> vaddr = plane_composer->map[0].vaddr;
>
> @@ -168,6 +168,8 @@ static void compose_plane(struct vkms_composer
> *primary_composer,
> pixel_blend = _blend;
>
> blend(vaddr_out, vaddr, primary_composer, plane_composer, pixel_blend);
> +
> + return 0;
> }
>
> static int compose_active_planes(void **vaddr_out,
> @@ -177,7 +179,7 @@ static int compose_active_planes(void **vaddr_out,
> struct drm_framebuffer *fb = _composer->fb;
> struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
> const void *vaddr;
> - int i;
> + int i, ret;
>
> if (!*vaddr_out) {
> *vaddr_out = kzalloc(gem_obj->size, GFP_KERNEL);
> @@ -187,8 +189,10 @@ static int compose_active_planes(void **vaddr_out,
> }
> }
>
> - if (WARN_ON(iosys_map_is_null(_composer->map[0])))
> + if (WARN_ON(iosys_map_is_null(_composer->map[0]))) {
> + DRM_DEBUG_DRIVER("Failed to compose. Invalid map in the primary
> plane.");
> return -EINVAL;
yes, good to have a debug msg here. I would say `mapping`
> + }
>
> vaddr = primary_composer->map[0].vaddr;
>
> @@ -198,10 +202,16 @@ static int compose_active_planes(void **vaddr_out,
>* planes should be in z-order and compose them associatively:
>* ((primary <- overlay) <- cursor)
>*/
> - for (i = 1; i < crtc_state->num_active_planes; i++)
> - compose_plane(primary_composer,
> - crtc_state->active_planes[i]->composer,
> - *vaddr_out);
> + for (i = 1; i < crtc_state->num_active_planes; i++) {
> + ret = compose_plane(primary_composer,
> + crtc_state->active_planes[i]->composer,
> + *vaddr_out);
tbh, I'm not sure on changing compose_plane behaviour here to
invalidate all composition in case of a invalid active plane mapping.
Warning about an inconsistent composition makes sense to me, but it
would be better if we can prevent all resources consumption around each plane
composition by checking the issue as soon as possible. One idea would be doing
this validation at the time of active_plane selection. Another would be just
check all active_plane mapping right before this loop.
What do you think?
> + if (ret) {
> + DRM_DEBUG_DRIVER("Failed to compose. Invalid map in the
> active_planes[%d].",
> + i);
> + return ret;
> + }
> + }
>
> return 0;
> }
> --
> 2.35.1
>
signature.asc
Description: PGP signature
Re: [PATCH v4 11/15] drm/shmem-helper: Add generic memory shrinker
On 5/11/22 22:09, Daniel Vetter wrote: > On Wed, May 11, 2022 at 07:06:18PM +0300, Dmitry Osipenko wrote: >> On 5/11/22 16:09, Daniel Vetter wrote: >>> I'd like to ask you to reduce the scope of the patchset and build the >>> shrinker only for virtio-gpu. I know that I first suggested to build >>> upon shmem helpers, but it seems that it's easier to do that in a later >>> patchset. >> The first version of the VirtIO shrinker didn't support memory eviction. >> Memory eviction support requires page fault handler to be aware of the >> evicted pages, what should we do about it? The page fault handling is a >> part of memory management, hence to me drm-shmem is already kinda a MM. > Hm I still don't get that part, why does that also not go through the > shmem helpers? The drm_gem_shmem_vm_ops includes the page faults handling, it's a helper by itself that is used by DRM drivers. I could try to move all the shrinker logic to the VirtIO and re-invent virtio_gem_shmem_vm_ops, but what is the point of doing this for each driver if we could have it once and for all in the common drm-shmem code? Maybe I should try to factor out all the shrinker logic from drm-shmem into a new drm-shmem-shrinker that could be shared by drivers? Will you be okay with this option? >>> I think we're talking past each another a bit. I'm only bringing up the >>> purge vs eviction topic we discussed in the other subthread again. >> >> Thomas asked to move the whole shrinker code to the VirtIO driver and >> I's saying that this is not a great idea to me, or am I misunderstanding >> the Thomas' suggestion? Thomas? > > I think it was just me creating a confusion here. > > fwiw I do also think that shrinker in shmem helpers makes sense, just in > case that was also lost in confusion. Okay, good that we're on the same page now. > I'm still confused why drivers need to know the difference > between evition and purging. Or maybe I'm confused again. Example: If userspace uses IOV addresses, then these addresses must be kept reserved while buffer is evicted. If BO is purged, then we don't need to retain the IOV space allocated for the purged BO. >>> Yeah but is that actually needed by anyone? If userspace fails to allocate >>> another bo because of lack of gpu address space then it's very easy to >>> handle that: >>> >>> 1. Make a rule that "out of gpu address space" gives you a special errno >>> code like ENOSPC >>> >>> 2. If userspace gets that it walks the list of all buffers it marked as >>> purgeable and nukes them (whether they have been evicted or not). Then it >>> retries the bo allocation. >>> >>> Alternatively you can do step 2 also directly from the bo alloc ioctl in >>> step 1. Either way you clean up va space, and actually a lot more (you >>> potentially nuke all buffers marked as purgeable, not just the ones that >>> have been purged already) and only when va cleanup is actually needed >>> >>> Trying to solve this problem at eviction time otoh means: >>> - we have this difference between eviction and purging >>> - it's still not complete, you still need to glue step 2 above into your >>> driver somehow, and once step 2 above is glued in doing additional >>> cleanup in the purge function is just duplicated logic >>> >>> So at least in my opinion this isn't the justification we need. And we >>> should definitely not just add that complication "in case, for the >>> future", if we don't have a real need right now. Adding it later on is >>> easy, removing it later on because it just gets in the way and confuses is >>> much harder. >> >> The IOVA space is only one example. >> >> In case of the VirtIO driver, we may have two memory allocation for a >> BO. One is the shmem allcation in guest and the other is in host's vram. >> If we will only release the guest's memory on purge, then the vram will >> remain allocated until BO is destroyed, which unnecessarily sub-optimal. > > Hm but why don't you just nuke the memory on the host side too when you > evict? Allowing the guest memory to be swapped out while keeping the host > memory allocation alive also doesn't make a lot of sense for me. Both can > be recreated (I guess at least?) on swap-in. Shouldn't be very doable or at least worth the efforts. It's userspace that manages data uploading, kernel only provides transport for the virtio-gpu commands. Drivers are free to use the same function for both purge() and evict() callbacks if they want. Getting rid of the purge() callback creates more problems than solves, IMO. -- Best regards, Dmitry
Re: [PATCH 1/5] dma-buf: cleanup dma_fence_unwrap selftest v2
Hey Daniel,
a gentle ping on this here. Those patches come before my drm-exec work,
so would be nice if we could get that reviewed first.
Thanks,
Christian.
Am 06.05.22 um 16:11 schrieb Christian König:
I had to send this out once more.
This time with the right mail addresses and a much simplified patch #3.
Christian.
Am 06.05.22 um 16:10 schrieb Christian König:
The selftests, fix the error handling, remove unused functions and stop
leaking memory in failed tests.
v2: fix the memory leak correctly.
Signed-off-by: Christian König
---
drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++
1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c
b/drivers/dma-buf/st-dma-fence-unwrap.c
index 039f016b57be..e20c5a7dcfe4 100644
--- a/drivers/dma-buf/st-dma-fence-unwrap.c
+++ b/drivers/dma-buf/st-dma-fence-unwrap.c
@@ -4,27 +4,19 @@
* Copyright (C) 2022 Advanced Micro Devices, Inc.
*/
+#include
+#include
+#include
#include
-#if 0
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#endif
#include "selftest.h"
#define CHAIN_SZ (4 << 10)
-static inline struct mock_fence {
+struct mock_fence {
struct dma_fence base;
spinlock_t lock;
-} *to_mock_fence(struct dma_fence *f) {
- return container_of(f, struct mock_fence, base);
-}
+};
static const char *mock_name(struct dma_fence *f)
{
@@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void)
return NULL;
spin_lock_init(>lock);
- dma_fence_init(>base, _ops, >lock, 0, 0);
+ dma_fence_init(>base, _ops, >lock,
+ dma_fence_context_alloc(1), 1);
return >base;
}
@@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int
num_fences, ...)
fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL);
if (!fences)
- return NULL;
+ goto error_put;
va_start(valist, num_fences);
for (i = 0; i < num_fences; ++i)
@@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int
num_fences, ...)
dma_fence_context_alloc(1),
1, false);
if (!array)
- goto cleanup;
+ goto error_free;
return >base;
-cleanup:
- for (i = 0; i < num_fences; ++i)
- dma_fence_put(fences[i]);
+error_free:
kfree(fences);
+
+error_put:
+ va_start(valist, num_fences);
+ for (i = 0; i < num_fences; ++i)
+ dma_fence_put(va_arg(valist, typeof(*fences)));
+ va_end(valist);
return NULL;
}
@@ -113,7 +110,6 @@ static int sanitycheck(void *arg)
if (!chain)
return -ENOMEM;
- dma_fence_signal(f);
dma_fence_put(chain);
return err;
}
@@ -154,10 +150,8 @@ static int unwrap_array(void *arg)
err = -EINVAL;
}
- dma_fence_signal(f1);
- dma_fence_signal(f2);
dma_fence_put(array);
- return 0;
+ return err;
}
static int unwrap_chain(void *arg)
@@ -196,10 +190,8 @@ static int unwrap_chain(void *arg)
err = -EINVAL;
}
- dma_fence_signal(f1);
- dma_fence_signal(f2);
dma_fence_put(chain);
- return 0;
+ return err;
}
static int unwrap_chain_array(void *arg)
@@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg)
err = -EINVAL;
}
- dma_fence_signal(f1);
- dma_fence_signal(f2);
dma_fence_put(chain);
- return 0;
+ return err;
}
int dma_fence_unwrap(void)
Re: [REPORT] syscall reboot + umh + firmware fallback
Tejun wrote: > Hello, Hello, > I'm not sure I'm reading it correctly but it looks like "process B" column I think you're interpreting the report correctly. > is superflous given that it's waiting on the same lock to do the same thing > that A is already doing (besides, you can't really halt the machine twice). Indeed! I've been in a daze. I thought kernel_halt() can be called twice by two different purposes. Sorry for the noise. > What it's reporting seems to be ABBA deadlock between A waiting on > umhelper_sem and C waiting on fw_st->completion. The report seems spurious: > > 1. wait_for_completion_killable_timeout() doesn't need someone to wake it up >to make forward progress because it will unstick itself after timeout >expires. I have a question about this one. Yes, it would never been stuck thanks to timeout. However, IIUC, timeouts are not supposed to expire in normal cases. So I thought a timeout expiration means not a normal case so need to inform it in terms of dependency so as to prevent further expiraton. That's why I have been trying to track even timeout'ed APIs. Do you think DEPT shouldn't track timeout APIs? If I was wrong, I shouldn't track the timeout APIs any more. > 2. complete_all() from __fw_load_abort() isn't the only source of wakeup. >The fw loader can be, and mainly should be, woken up by firmware loading >actually completing instead of being aborted. This is the point I'd like to ask. In normal cases, fw_load_done() might happen, of course, if the loading gets completed. However, I was wondering if the kernel ensures either fw_load_done() or fw_load_abort() to be called by *another* context while kernel_halt(). > Thanks. Thank you very much! Byungchul > > -- > tejun >
Re: [PATCH 7/7] drm/mgag200: Split up connector's mode_valid helper
Hi
Am 12.05.22 um 12:38 schrieb Jocelyn Falempe:
On 09/05/2022 12:35, Thomas Zimmermann wrote:
Split up the connector's mode_valid helper into a simple-pipe and a
mode-config helper. The simple-pipe helper tests for display-size
limits while the mode-config helper tests for memory-bandwidth limits.
Also add the mgag200_ prefix to mga_vga_calculate_mode_bandwidth() and
comment on the function's purpose.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/mgag200/mgag200_mode.c | 146 -
1 file changed, 69 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 92d3ae9489f0..a808827d7a2c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -681,38 +681,27 @@ static int
mgag200_vga_connector_helper_get_modes(struct drm_connector *connecto
return ret;
}
-static uint32_t mga_vga_calculate_mode_bandwidth(struct
drm_display_mode *mode,
- int bits_per_pixel)
-{
- uint32_t total_area, divisor;
- uint64_t active_area, pixels_per_second, bandwidth;
- uint64_t bytes_per_pixel = (bits_per_pixel + 7) / 8;
-
- divisor = 1024;
-
- if (!mode->htotal || !mode->vtotal || !mode->clock)
- return 0;
-
- active_area = mode->hdisplay * mode->vdisplay;
- total_area = mode->htotal * mode->vtotal;
-
- pixels_per_second = active_area * mode->clock * 1000;
- do_div(pixels_per_second, total_area);
-
- bandwidth = pixels_per_second * bytes_per_pixel * 100;
- do_div(bandwidth, divisor);
+static const struct drm_connector_helper_funcs
mga_vga_connector_helper_funcs = {
+ .get_modes = mgag200_vga_connector_helper_get_modes,
+};
- return (uint32_t)(bandwidth);
-}
+static const struct drm_connector_funcs mga_vga_connector_funcs = {
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .atomic_duplicate_state =
drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
-#define MODE_BANDWIDTH MODE_BAD
+/*
+ * Simple Display Pipe
+ */
-static enum drm_mode_status mga_vga_mode_valid(struct drm_connector
*connector,
- struct drm_display_mode *mode)
+static enum drm_mode_status
+mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe
*pipe,
+ const struct drm_display_mode *mode)
{
- struct drm_device *dev = connector->dev;
- struct mga_device *mdev = to_mga_device(dev);
- int bpp = 32;
+ struct mga_device *mdev = to_mga_device(pipe->crtc.dev);
if (IS_G200_SE(mdev)) {
u32 unique_rev_id = mdev->model.g200se.unique_rev_id;
@@ -722,42 +711,17 @@ static enum drm_mode_status
mga_vga_mode_valid(struct drm_connector *connector,
return MODE_VIRTUAL_X;
if (mode->vdisplay > 1200)
return MODE_VIRTUAL_Y;
- if (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (24400 * 1024))
- return MODE_BANDWIDTH;
} else if (unique_rev_id == 0x02) {
if (mode->hdisplay > 1920)
return MODE_VIRTUAL_X;
if (mode->vdisplay > 1200)
return MODE_VIRTUAL_Y;
- if (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (30100 * 1024))
- return MODE_BANDWIDTH;
- } else {
- if (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (55000 * 1024))
- return MODE_BANDWIDTH;
}
} else if (mdev->type == G200_WB) {
if (mode->hdisplay > 1280)
return MODE_VIRTUAL_X;
if (mode->vdisplay > 1024)
return MODE_VIRTUAL_Y;
- if (mga_vga_calculate_mode_bandwidth(mode, bpp) >
- (31877 * 1024))
- return MODE_BANDWIDTH;
- } else if (mdev->type == G200_EV &&
- (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (32700 * 1024))) {
- return MODE_BANDWIDTH;
- } else if (mdev->type == G200_EH &&
- (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (37500 * 1024))) {
- return MODE_BANDWIDTH;
- } else if (mdev->type == G200_ER &&
- (mga_vga_calculate_mode_bandwidth(mode,
- bpp) > (55000 * 1024))) {
- return MODE_BANDWIDTH;
}
if ((mode->hdisplay % 8) != 0 || (mode->hsync_start % 8) != 0 ||
@@ -775,30 +739,6 @@ static enum drm_mode_status
mga_vga_mode_valid(struct drm_connector *connector,
return MODE_OK;
}
-static const struct drm_connector_helper_funcs
mga_vga_connector_helper_funcs = {
- .get_modes = mgag200_vga_connector_helper_get_modes,
- .mode_valid = mga_vga_mode_valid,
-};
-
-static const struct drm_connector_funcs mga_vga_connector_funcs
Re: [PATCH v2 3/4] soc: visconti: Add Toshiba Visconti AFFINE image processing accelerator
On 4/27/22 15:23, Yuji Ishikawa wrote:
> Adds support to AFFINE image processing accelerator on Toshiba Visconti ARM
> SoCs.
> This accelerator supoorts affine transform, lens undistortion and LUT
> transform.
>
> Signed-off-by: Yuji Ishikawa
> Reviewed-by: Nobuhiro Iwamatsu
> ---
> v1 -> v2:
> - apply checkpatch.pl --strict
> - renamed identifiers; hwd_AFFINE_ to hwd_affine_
> ---
> drivers/soc/visconti/Kconfig | 6 +
> drivers/soc/visconti/Makefile| 2 +
> drivers/soc/visconti/affine/Makefile | 6 +
> drivers/soc/visconti/affine/affine.c | 451 +++
> drivers/soc/visconti/affine/hwd_affine.c | 206 +
> drivers/soc/visconti/affine/hwd_affine.h | 83
> drivers/soc/visconti/affine/hwd_affine_reg.h | 45 ++
> drivers/soc/visconti/uapi/affine.h | 87
> 8 files changed, 886 insertions(+)
> create mode 100644 drivers/soc/visconti/affine/Makefile
> create mode 100644 drivers/soc/visconti/affine/affine.c
> create mode 100644 drivers/soc/visconti/affine/hwd_affine.c
> create mode 100644 drivers/soc/visconti/affine/hwd_affine.h
> create mode 100644 drivers/soc/visconti/affine/hwd_affine_reg.h
> create mode 100644 drivers/soc/visconti/uapi/affine.h
>
> diff --git a/drivers/soc/visconti/Kconfig b/drivers/soc/visconti/Kconfig
> index 8b1378917..01583d407 100644
> --- a/drivers/soc/visconti/Kconfig
> +++ b/drivers/soc/visconti/Kconfig
> @@ -1 +1,7 @@
> +if ARCH_VISCONTI
> +
> +config VISCONTI_AFFINE
> +bool "Visconti Affine driver"
> +
> +endif
>
> diff --git a/drivers/soc/visconti/Makefile b/drivers/soc/visconti/Makefile
> index 8d710da08..b25a726c3 100644
> --- a/drivers/soc/visconti/Makefile
> +++ b/drivers/soc/visconti/Makefile
> @@ -4,3 +4,5 @@
> #
>
> obj-y += ipa_common.o
> +
> +obj-$(CONFIG_VISCONTI_AFFINE) += affine/
> diff --git a/drivers/soc/visconti/affine/Makefile
> b/drivers/soc/visconti/affine/Makefile
> new file mode 100644
> index 0..82f83b2d6
> --- /dev/null
> +++ b/drivers/soc/visconti/affine/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for the Visconti AFFINE driver
> +#
> +
> +obj-y += affine.o hwd_affine.o
> diff --git a/drivers/soc/visconti/affine/affine.c
> b/drivers/soc/visconti/affine/affine.c
> new file mode 100644
> index 0..eea045dcf
> --- /dev/null
> +++ b/drivers/soc/visconti/affine/affine.c
> @@ -0,0 +1,451 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +/* Toshiba Visconti Affine Accelerator Support
> + *
> + * (C) Copyright 2022 TOSHIBA CORPORATION
> + * (C) Copyright 2022 Toshiba Electronic Devices & Storage Corporation
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "hwd_affine.h"
> +#include "../ipa_common.h"
> +#include "../uapi/affine.h"
> +
> +struct affine_priv {
> + struct device *dev;
> + struct miscdevice miscdev;
> + struct mutex lock;
> + void __iomem *regs;
> + int irq;
> + wait_queue_head_t waitq;
> + enum drv_ipa_state status;
> + unsigned int hwd_event;
> + unsigned int poll_event;
> + int id;
> + char name[16];
> + bool dma_coherent;
> + struct hwd_affine_status hwd_status;
> +
> + struct dma_buf_attachment *dba[DRV_AFFINE_BUFFER_INDEX_MAX];
> + struct sg_table *sgt[DRV_AFFINE_BUFFER_INDEX_MAX];
> + enum dma_data_direction dma_dir[DRV_AFFINE_BUFFER_INDEX_MAX];
> + unsigned int dma_count;
> +
> + dma_addr_t buffer_iova[DRV_AFFINE_BUFFER_INDEX_MAX];
> +};
> +
> +static u32 affine_ipa_addr_to_iova(struct affine_priv *priv, struct
> drv_ipa_addr addr)
> +{
> + u32 iova = 0;
> +
> + if (addr.buffer_index < priv->dma_count &&
> + addr.offset < priv->dba[addr.buffer_index]->dmabuf->size)
> + iova = priv->buffer_iova[addr.buffer_index] + addr.offset;
> + return iova;
> +}
> +
> +static int affine_attach_dma_buf(struct affine_priv *priv, unsigned int
> buffer_index,
> + struct drv_ipa_buffer_info *buffer_info)
> +{
> + int ret = 0;
> + dma_addr_t addr;
> +
> + if (buffer_index >= DRV_AFFINE_BUFFER_INDEX_MAX) {
> + dev_err(priv->dev, "Buffer index invalid: index=%d\n",
> buffer_index);
> + return -EINVAL;
> + }
> +
> + switch (buffer_info[buffer_index].direction) {
> + case DRV_IPA_DIR_NONE:
> + priv->dma_dir[priv->dma_count] = DMA_NONE;
> + break;
> + case DRV_IPA_DIR_TO_DEVICE:
> + priv->dma_dir[priv->dma_count] = DMA_TO_DEVICE;
> + break;
> + case DRV_IPA_DIR_FROM_DEVICE:
> + priv->dma_dir[priv->dma_count] = DMA_FROM_DEVICE;
> + break;
> + case DRV_IPA_DIR_BIDIRECTION:
> + priv->dma_dir[priv->dma_count] = DMA_BIDIRECTIONAL;
> + break;
> +
Re: [PATCH] drm/meson: Fix refcount leak in meson_encoder_hdmi_init
Hi,
On 2022/5/12 17:32, Neil Armstrong wrote:
> Hi,
>
> On 12/05/2022 11:21, Miaoqian Lin wrote:
>> of_find_device_by_node() takes a reference to the embedded struct device,
>> we should use put_device() to release it when not need anymore.
>> Add missing put_device() in error path to avoid refcount leak.
>>
>> Fixes: 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
>> DRM_BRIDGE_ATTACH_NO_CONNECTOR")
>> Signed-off-by: Miaoqian Lin
>
> You already sent the same patch yesterday, please avoid this.
>
Sorry for the mistake, I realized this after I sent it. I will be more careful.
> Neil
>
>> ---
>> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
>> b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
>> index 5e306de6f485..de87f02cd388 100644
>> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
>> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
>> @@ -435,8 +435,10 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
>> cec_fill_conn_info_from_drm(_info,
>> meson_encoder_hdmi->connector);
>> notifier = cec_notifier_conn_register(>dev, NULL,
>> _info);
>> - if (!notifier)
>> + if (!notifier) {
>> + put_device(>dev);
>> return -ENOMEM;
>> + }
>> meson_encoder_hdmi->cec_notifier = notifier;
>> }
>
Re: [PATCH 0/4] Add Toshiba Visconti DNN image processing accelerator driver
Hi Yuji, On 4/28/22 15:11, Yuji Ishikawa wrote: > This series is the DNN image processing accelerator driver for Toshiba's ARM > SoC, Visconti[0]. > This provides DT binding documentation, device driver, MAINTAINER files. > > The second patch "soc: visconti: Add Toshiba Visconti image processing > accelerator common source" > and the fourth patch "MAINTAINERS: ..." are the same as the ones in the > preceding post for affine driver. There appears to be no documentation whatsoever, unless I am missing something. How is the uAPI supposed to be used? What does it do? What formats does it accept or produce? If this processes images, then (as Laurent mentioned) this is more suitable as a V4L2 mem2mem driver. See https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dev-mem2mem.html and the many drivers in drivers/media that use it (git grep v4l2-mem2mem.h). But without any explanation whatsoever I have no idea what does or does not make sense. Regards, Hans > > Best regards, > Yuji > > [0]: > https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-recognition-processors-visconti.html > > Yuji Ishikawa (4): > dt-bindings: soc: visconti: Add Toshiba Visconti DNN image processing > accelerator bindings > soc: visconti: Add Toshiba Visconti image processing accelerator > common source > soc: visconti: Add Toshiba Visconti DNN image processing accelerator > MAINTAINERS: Add entries for Toshiba Visconti DNN image processing > accelerator > > .../soc/visconti/toshiba,visconti-dnn.yaml| 54 ++ > MAINTAINERS | 2 + > drivers/soc/Kconfig | 1 + > drivers/soc/Makefile | 1 + > drivers/soc/visconti/Kconfig | 7 + > drivers/soc/visconti/Makefile | 8 + > drivers/soc/visconti/dnn/Makefile | 6 + > drivers/soc/visconti/dnn/dnn.c| 533 ++ > drivers/soc/visconti/dnn/hwd_dnn.c| 183 ++ > drivers/soc/visconti/dnn/hwd_dnn.h| 68 +++ > drivers/soc/visconti/dnn/hwd_dnn_reg.h| 228 > drivers/soc/visconti/ipa_common.c | 55 ++ > drivers/soc/visconti/ipa_common.h | 18 + > drivers/soc/visconti/uapi/dnn.h | 77 +++ > drivers/soc/visconti/uapi/ipa.h | 88 +++ > 15 files changed, 1329 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/soc/visconti/toshiba,visconti-dnn.yaml > create mode 100644 drivers/soc/visconti/Kconfig > create mode 100644 drivers/soc/visconti/Makefile > create mode 100644 drivers/soc/visconti/dnn/Makefile > create mode 100644 drivers/soc/visconti/dnn/dnn.c > create mode 100644 drivers/soc/visconti/dnn/hwd_dnn.c > create mode 100644 drivers/soc/visconti/dnn/hwd_dnn.h > create mode 100644 drivers/soc/visconti/dnn/hwd_dnn_reg.h > create mode 100644 drivers/soc/visconti/ipa_common.c > create mode 100644 drivers/soc/visconti/ipa_common.h > create mode 100644 drivers/soc/visconti/uapi/dnn.h > create mode 100644 drivers/soc/visconti/uapi/ipa.h >
Re: (subset) [PATCH] MAINTAINERS: add Melissa to V3D maintainers
On 05/12, Maxime Ripard wrote: > On Fri, 29 Apr 2022 18:33:30 -0100, Melissa Wen wrote: > > I've been contributing to v3d through improvements, reviews, testing, > > debugging, etc. So, I'm adding myself as a co-maintainer of the V3D > > driver. > > > > > > Applied to drm/drm-misc (drm-misc-next). Thanks :) > > Thanks! > Maxime signature.asc Description: PGP signature
Re: [bug report] drm/msm: devcoredump iommu fault support
On Mon, May 09, 2022 at 07:48:23AM -0700, Rob Clark wrote:
> On Sun, May 8, 2022 at 11:28 PM Dan Carpenter
> wrote:
> > 407 } else {
> > 408 /*
> > 409 * We couldn't attribute this fault to any
> > particular context,
> > 410 * so increment the global fault count instead.
> > 411 */
> > 412 gpu->global_faults++;
> > 413 }
> > 414
> > 415 /* Record the crash state */
> > 416 pm_runtime_get_sync(>pdev->dev);
> > 417 msm_gpu_crashstate_capture(gpu, submit, comm, cmd);
> > ^^^
> > This function calls:
> >
> > dev_coredumpm(gpu->dev->dev, THIS_MODULE, gpu, 0, GFP_KERNEL,
> > ^^^
> > Which kfrees gpu.
>
> How does the gpu object get kfree'd? That is the root problem, it
> shouldn't be freed until module unload. I don't think e25e92e08e32:
> "drm/msm: devcoredump iommu fault support" is actually related.
>
> Is there a way to reproduce this?
Ah. Thanks for your feedback. I saw free(data) and misread it as
kfree(data). It's actually a function pointer which is
msm_gpu_devcoredump_free() so it doesn't free "gpu".
My bad.
regards,
dan carpenter
Re: [PATCH 13/14] drm/vc4: crtc: Fix out of order frames during asynchronous page flips
On 05/09, Melissa Wen wrote:
> O 05/09, Melissa Wen wrote:
> > On 05/03, Maxime Ripard wrote:
> > > When doing an asynchronous page flip (PAGE_FLIP ioctl with the
> > > DRM_MODE_PAGE_FLIP_ASYNC flag set), the current code waits for the
> > > possible GPU buffer being rendered through a call to
> > > vc4_queue_seqno_cb().
> > >
> > > On the BCM2835-37, the GPU driver is part of the vc4 driver and that
> > > function is defined in vc4_gem.c to wait for the buffer to be rendered,
> > > and once it's done, call a callback.
> > >
> > > However, on the BCM2711 used on the RaspberryPi4, the GPU driver is
> > > separate (v3d) and that function won't do anything. This was working
> > > because we were going into a path, due to uninitialized variables, that
> > > was always scheduling the callback.
> > >
> > > However, we were never actually waiting for the buffer to be rendered
> > > which was resulting in frames being displayed out of order.
> > >
> > > The generic API to signal those kind of completion in the kernel are the
> > > DMA fences, and fortunately the v3d drivers supports them and signal
> > > when its job is done. That API also provides an equivalent function that
> > > allows to have a callback being executed when the fence is signalled as
> > > done.
> > >
> > > Let's change our driver a bit to rely on the previous function for the
> > > older SoCs, and on DMA fences for the BCM2711.
> > >
> > > Signed-off-by: Maxime Ripard
> > > ---
> > > drivers/gpu/drm/vc4/vc4_crtc.c | 41 --
> > > 1 file changed, 39 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c
> > > b/drivers/gpu/drm/vc4/vc4_crtc.c
> > > index e0ae7bef08fa..8e1369fca937 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> > > @@ -776,6 +776,7 @@ struct vc4_async_flip_state {
> > > struct drm_pending_vblank_event *event;
> > >
> > > union {
> > > + struct dma_fence_cb fence;
> > > struct vc4_seqno_cb seqno;
> > > } cb;
> > > };
> > > @@ -835,6 +836,43 @@ static void
> > > vc4_async_page_flip_seqno_complete(struct vc4_seqno_cb *cb)
> > > vc4_bo_dec_usecnt(bo);
> > > }
> > >
> > > +static void vc4_async_page_flip_fence_complete(struct dma_fence *fence,
> > > +struct dma_fence_cb *cb)
> > > +{
> > > + struct vc4_async_flip_state *flip_state =
> > > + container_of(cb, struct vc4_async_flip_state, cb.fence);
> > > +
> > > + vc4_async_page_flip_complete(flip_state);
> > > + dma_fence_put(fence);
> > > +}
> > > +
> > > +static int vc4_async_set_fence_cb(struct drm_device *dev,
> > > + struct vc4_async_flip_state *flip_state)
> > > +{
> > > + struct drm_framebuffer *fb = flip_state->fb;
> > > + struct drm_gem_cma_object *cma_bo = drm_fb_cma_get_gem_obj(fb, 0);
> > > + struct vc4_dev *vc4 = to_vc4_dev(dev);
> > > + struct dma_fence *fence;
> > > + int ret;
> > > +
> > > + if (!vc4->is_vc5) {
> > > + struct vc4_bo *bo = to_vc4_bo(_bo->base);
> > > +
> > > + return vc4_queue_seqno_cb(dev, _state->cb.seqno, bo->seqno,
> > > + vc4_async_page_flip_seqno_complete);
> > > + }
> > > +
> > > + ret = dma_resv_get_singleton(cma_bo->base.resv, false, );
> + for kernel bot complaint, I replaced false with `DMA_RESV_USAGE_READ`
> to run some tests
>
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (dma_fence_add_callback(fence, _state->cb.fence,
me again :)
I was thinking if we should add a check here for !fence and just complete the
page flip,
instead of letting `dma_fence_add_callback` warns whenever fence is NULL.
I think there are situation in which fence is NULL and it is not an
issue, right? Does it make sense?
> > > +vc4_async_page_flip_fence_complete))
> > > + vc4_async_page_flip_fence_complete(fence,
> > > _state->cb.fence);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int
> > > vc4_async_page_flip_common(struct drm_crtc *crtc,
> > > struct drm_framebuffer *fb,
> > > @@ -874,8 +912,7 @@ vc4_async_page_flip_common(struct drm_crtc *crtc,
> > >*/
> > > drm_atomic_set_fb_for_plane(plane->state, fb);
> > >
> > > - vc4_queue_seqno_cb(dev, _state->cb.seqno, bo->seqno,
> > > -vc4_async_page_flip_seqno_complete);
> > > + vc4_async_set_fence_cb(dev, flip_state);
> >
> > I tried to run igt kms_async_flip, but the test still seems very tied to
> > the i915 driver. So, as far as I could check, I didn't see any red
> > flags and sync page flip behaviour seems preserved.
> >
> > >
> > > /* Driver takes ownership of state on successful async commit. */
> > > return 0;
> > > --
> > > 2.35.1
> > >
>
>
signature.asc
Description: PGP signature
Re: [PATCH 7/7] drm/mgag200: Split up connector's mode_valid helper
On 09/05/2022 12:35, Thomas Zimmermann wrote:
Split up the connector's mode_valid helper into a simple-pipe and a
mode-config helper. The simple-pipe helper tests for display-size
limits while the mode-config helper tests for memory-bandwidth limits.
Also add the mgag200_ prefix to mga_vga_calculate_mode_bandwidth() and
comment on the function's purpose.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/mgag200/mgag200_mode.c | 146 -
1 file changed, 69 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 92d3ae9489f0..a808827d7a2c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -681,38 +681,27 @@ static int mgag200_vga_connector_helper_get_modes(struct
drm_connector *connecto
return ret;
}
-static uint32_t mga_vga_calculate_mode_bandwidth(struct drm_display_mode *mode,
- int bits_per_pixel)
-{
- uint32_t total_area, divisor;
- uint64_t active_area, pixels_per_second, bandwidth;
- uint64_t bytes_per_pixel = (bits_per_pixel + 7) / 8;
-
- divisor = 1024;
-
- if (!mode->htotal || !mode->vtotal || !mode->clock)
- return 0;
-
- active_area = mode->hdisplay * mode->vdisplay;
- total_area = mode->htotal * mode->vtotal;
-
- pixels_per_second = active_area * mode->clock * 1000;
- do_div(pixels_per_second, total_area);
-
- bandwidth = pixels_per_second * bytes_per_pixel * 100;
- do_div(bandwidth, divisor);
+static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs
= {
+ .get_modes = mgag200_vga_connector_helper_get_modes,
+};
- return (uint32_t)(bandwidth);
-}
+static const struct drm_connector_funcs mga_vga_connector_funcs = {
+ .reset = drm_atomic_helper_connector_reset,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy= drm_connector_cleanup,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
-#define MODE_BANDWIDTH MODE_BAD
+/*
+ * Simple Display Pipe
+ */
-static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
-struct drm_display_mode *mode)
+static enum drm_mode_status
+mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
+ const struct drm_display_mode *mode)
{
- struct drm_device *dev = connector->dev;
- struct mga_device *mdev = to_mga_device(dev);
- int bpp = 32;
+ struct mga_device *mdev = to_mga_device(pipe->crtc.dev);
if (IS_G200_SE(mdev)) {
u32 unique_rev_id = mdev->model.g200se.unique_rev_id;
@@ -722,42 +711,17 @@ static enum drm_mode_status mga_vga_mode_valid(struct
drm_connector *connector,
return MODE_VIRTUAL_X;
if (mode->vdisplay > 1200)
return MODE_VIRTUAL_Y;
- if (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (24400 * 1024))
- return MODE_BANDWIDTH;
} else if (unique_rev_id == 0x02) {
if (mode->hdisplay > 1920)
return MODE_VIRTUAL_X;
if (mode->vdisplay > 1200)
return MODE_VIRTUAL_Y;
- if (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (30100 * 1024))
- return MODE_BANDWIDTH;
- } else {
- if (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (55000 * 1024))
- return MODE_BANDWIDTH;
}
} else if (mdev->type == G200_WB) {
if (mode->hdisplay > 1280)
return MODE_VIRTUAL_X;
if (mode->vdisplay > 1024)
return MODE_VIRTUAL_Y;
- if (mga_vga_calculate_mode_bandwidth(mode, bpp) >
- (31877 * 1024))
- return MODE_BANDWIDTH;
- } else if (mdev->type == G200_EV &&
- (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (32700 * 1024))) {
- return MODE_BANDWIDTH;
- } else if (mdev->type == G200_EH &&
- (mga_vga_calculate_mode_bandwidth(mode, bpp)
- > (37500 * 1024))) {
- return MODE_BANDWIDTH;
- } else if (mdev->type == G200_ER &&
- (mga_vga_calculate_mode_bandwidth(mode,
- bpp) > (55000 * 1024))) {
- return MODE_BANDWIDTH;
Re: [PATCH v2] mgag200: Enable atomic gamma lut update
On 12/05/2022 10:52, Thomas Zimmermann wrote:
Hi
Am 11.05.22 um 17:28 schrieb Jocelyn Falempe:
Add support for atomic update of gamma lut.
With this patch the "Night light" feature of gnome3
is working properly on mgag200.
v2:
- Add a default linear gamma function
- renamed functions with mgag200 prefix
- use format's 4cc code instead of bit depth
- use better interpolation for 16bits gamma
- remove legacy function mga_crtc_load_lut()
- can't remove the call to drm_mode_crtc_set_gamma_size()
because it doesn't work with userspace.
- other small refactors
Signed-off-by: Jocelyn Falempe
I already gave a Tested-by on the first iteration. It's good practice to
add these tags in follow-up patches unless the patch has changed entirely.
Sorry, I though I changed too much code in v2 to add it.
A few more comments are below. With those fixed:
Reviewed-by: Thomas Zimmermann
I suggest to post another version of the patch and merge it if no
further comments arrive within 2 days.
---
drivers/gpu/drm/mgag200/mgag200_mode.c | 125 -
1 file changed, 81 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 6e18d3bbd720..b748bc5b0e93 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -32,57 +32,76 @@
* This file contains setup code for the CRTC.
*/
-static void mga_crtc_load_lut(struct drm_crtc *crtc)
+static void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
+ uint32_t format)
{
- struct drm_device *dev = crtc->dev;
- struct mga_device *mdev = to_mga_device(dev);
- struct drm_framebuffer *fb;
- u16 *r_ptr, *g_ptr, *b_ptr;
int i;
- if (!crtc->enabled)
- return;
-
- if (!mdev->display_pipe.plane.state)
- return;
+ WREG8(DAC_INDEX + MGA1064_INDEX, 0);
- fb = mdev->display_pipe.plane.state->fb;
+ switch (format) {
+ case DRM_FORMAT_RGB565:
+ /* Use better interpolation, to take 32 values from 0 to 255 */
+ for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+ }
+ /* Green has one more bit, so add padding with 0 for red and
blue. */
+ for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+ }
+ break;
+ case DRM_FORMAT_RGB888:
+ case DRM_FORMAT_XRGB:
+ for (i = 0; i < MGAG200_LUT_SIZE; i++) {
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+ }
These loops look much nicer to me.
+ break;
+ default:
+ drm_warn_once(>base, "Unsupported format for gamma
%d\n", format);
There's a print format modifier for 4cc formats. It's %p4cc and expects
a pointer to the format's 4cc code. See 'git grep p4cc' for examples.
ok, that's a cool feature.
The comment itself is not easy to understand. Maybe "Unsupported format
%p4cc for gamma correction.\n" ?
Sure, having good error message is always helpful.
+ break;
+ }
+}
- r_ptr = crtc->gamma_store;
- g_ptr = r_ptr + crtc->gamma_size;
- b_ptr = g_ptr + crtc->gamma_size;
+static void mgag200_crtc_set_gamma(struct mga_device *mdev,
+ struct drm_color_lut *lut,
+ uint32_t format)
+{
+ int i;
WREG8(DAC_INDEX + MGA1064_INDEX, 0);
- if (fb && fb->format->cpp[0] * 8 == 16) {
- int inc = (fb->format->depth == 15) ? 8 : 4;
- u8 r, b;
- for (i = 0; i < MGAG200_LUT_SIZE; i += inc) {
- if (fb->format->depth == 16) {
- if (i > (MGAG200_LUT_SIZE >> 1)) {
- r = b = 0;
- } else {
- r = *r_ptr++ >> 8;
- b = *b_ptr++ >> 8;
- r_ptr++;
- b_ptr++;
- }
- } else {
- r = *r_ptr++ >> 8;
- b = *b_ptr++ >> 8;
- }
- /* VGA registers */
- WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
+ switch (format) {
+ case DRM_FORMAT_RGB565:
+ /* Use better interpolation, to take 32 values from lut[0] to
lut[255] */
+ for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 8 + i / 4].red
>> 8);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 4 + i /
16].green >> 8);
+ WREG8(DAC_INDEX +
[PATCH] drm/i915: Fix vm use-after-free in vma destruction
In vma destruction, the following race may occur:
Thread 1: Thread 2:
i915_vma_destroy();
...
list_del_init(vma->vm_link);
...
mutex_unlock(vma->vm->mutex);
__i915_vm_release();
release_references();
And in release_reference() we dereference vma->vm to get to the
vm gt pointer, leadin go a use-after free.
However, __i915_vm_release() grabs the vm->mutex so the vm won't be
destroyed before vma->vm->mutex is released, so extract the gt pointer
under the vm->mutex to avoid the vma->vm dereference in
release_references().
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5944
Fixes: e1a7ab4fca ("drm/i915: Remove the vm open count")
Cc: Niranjana Vishwanathapura
Cc: Matthew Auld
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/i915_vma.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 4f6db539571a..5d6850a91a69 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1636,10 +1636,10 @@ static void force_unbind(struct i915_vma *vma)
GEM_BUG_ON(drm_mm_node_allocated(>node));
}
-static void release_references(struct i915_vma *vma, bool vm_ddestroy)
+static void release_references(struct i915_vma *vma, struct intel_gt *gt,
+ bool vm_ddestroy)
{
struct drm_i915_gem_object *obj = vma->obj;
- struct intel_gt *gt = vma->vm->gt;
GEM_BUG_ON(i915_vma_is_active(vma));
@@ -1694,11 +1694,12 @@ void i915_vma_destroy_locked(struct i915_vma *vma)
force_unbind(vma);
list_del_init(>vm_link);
- release_references(vma, false);
+ release_references(vma, vma->vm->gt, false);
}
void i915_vma_destroy(struct i915_vma *vma)
{
+ struct intel_gt *gt;
bool vm_ddestroy;
mutex_lock(>vm->mutex);
@@ -1706,8 +1707,11 @@ void i915_vma_destroy(struct i915_vma *vma)
list_del_init(>vm_link);
vm_ddestroy = vma->vm_ddestroy;
vma->vm_ddestroy = false;
+
+ /* vma->vm may be freed when releasing vma->vm->mutex. */
+ gt = vma->vm->gt;
mutex_unlock(>vm->mutex);
- release_references(vma, vm_ddestroy);
+ release_references(vma, gt, vm_ddestroy);
}
void i915_vma_parked(struct intel_gt *gt)
--
2.34.1
Re: [DKIM] [PATCH v12, 13/17] media: mediatek: vcodec: Extract H264 common code
Hi Yunfei,
On 5/12/22 04:19, Yunfei Dong wrote:
> Mt8192 can use some of common code with mt8183. Moves them to
> a new file in order to reuse.
>
> Signed-off-by: Yunfei Dong
> Reviewed-by: Nicolas Dufresne
> ---
> .../media/platform/mediatek/vcodec/Makefile | 1 +
> .../vcodec/vdec/vdec_h264_req_common.c| 310 +
> .../vcodec/vdec/vdec_h264_req_common.h| 274 +++
> .../mediatek/vcodec/vdec/vdec_h264_req_if.c | 427 ++
> 4 files changed, 629 insertions(+), 383 deletions(-)
> create mode 100644
> drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.c
> create mode 100644
> drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.h
>
> diff --git a/drivers/media/platform/mediatek/vcodec/Makefile
> b/drivers/media/platform/mediatek/vcodec/Makefile
> index 359619653a0e..3f41d748eee5 100644
> --- a/drivers/media/platform/mediatek/vcodec/Makefile
> +++ b/drivers/media/platform/mediatek/vcodec/Makefile
> @@ -9,6 +9,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \
> vdec/vdec_vp8_if.o \
> vdec/vdec_vp9_if.o \
> vdec/vdec_h264_req_if.o \
> + vdec/vdec_h264_req_common.o \
> mtk_vcodec_dec_drv.o \
> vdec_drv_if.o \
> vdec_vpu_if.o \
> diff --git
> a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.c
> b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.c
> new file mode 100644
> index ..4e7c9d47751d
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.c
> @@ -0,0 +1,310 @@
Here there is still a cast to iomem:
> +void mtk_vdec_h264_copy_scaling_matrix(struct slice_api_h264_scaling_matrix
> *dst_matrix,
> +const struct
> v4l2_ctrl_h264_scaling_matrix *src_matrix)
> +{
> + memcpy_toio((void __iomem *)dst_matrix->scaling_list_4x4,
> src_matrix->scaling_list_4x4,
> + sizeof(dst_matrix->scaling_list_4x4));
> +
> + memcpy_toio((void __iomem *)dst_matrix->scaling_list_8x8,
> src_matrix->scaling_list_8x8,
> + sizeof(dst_matrix->scaling_list_8x8));
> +}
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
> b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
> index 27119aa31dd9..b055ceea481d 100644
> --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_if.c
> -static void
> -get_h264_scaling_matrix(struct slice_api_h264_scaling_matrix *dst_matrix,
> - const struct v4l2_ctrl_h264_scaling_matrix *src_matrix)
> -{
> - memcpy(dst_matrix->scaling_list_4x4, src_matrix->scaling_list_4x4,
> -sizeof(dst_matrix->scaling_list_4x4));
> -
> - memcpy(dst_matrix->scaling_list_8x8, src_matrix->scaling_list_8x8,
> -sizeof(dst_matrix->scaling_list_8x8));
> -}
but that function was moved (AFAICT) from vdec_h264_req_if.c where a regular
memcpy was
used.
Did you miss one iomem case?
Can I change mtk_vdec_h264_copy_scaling_matrix() to:
void mtk_vdec_h264_copy_scaling_matrix(struct slice_api_h264_scaling_matrix
*dst_matrix,
const struct
v4l2_ctrl_h264_scaling_matrix *src_matrix)
{
memcpy(dst_matrix->scaling_list_4x4, src_matrix->scaling_list_4x4,
sizeof(dst_matrix->scaling_list_4x4));
memcpy(dst_matrix->scaling_list_8x8, src_matrix->scaling_list_8x8,
sizeof(dst_matrix->scaling_list_8x8));
}
If that's OK, then I'll do that manually, so no need to post a v13.
Everything else looks fine, so this is the only issue that needs to be resolved.
Regards,
Hans
Re: [PATCH] drm/meson: Fix refcount leak in meson_encoder_hdmi_init
Hi,
On 12/05/2022 11:21, Miaoqian Lin wrote:
of_find_device_by_node() takes a reference to the embedded struct device,
we should use put_device() to release it when not need anymore.
Add missing put_device() in error path to avoid refcount leak.
Fixes: 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
DRM_BRIDGE_ATTACH_NO_CONNECTOR")
Signed-off-by: Miaoqian Lin
You already sent the same patch yesterday, please avoid this.
Neil
---
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 5e306de6f485..de87f02cd388 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -435,8 +435,10 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
cec_fill_conn_info_from_drm(_info,
meson_encoder_hdmi->connector);
notifier = cec_notifier_conn_register(>dev, NULL, _info);
- if (!notifier)
+ if (!notifier) {
+ put_device(>dev);
return -ENOMEM;
+ }
meson_encoder_hdmi->cec_notifier = notifier;
}
[PATCH] drm/meson: Fix refcount leak in meson_encoder_hdmi_init
of_find_device_by_node() takes a reference to the embedded struct device,
we should use put_device() to release it when not need anymore.
Add missing put_device() in error path to avoid refcount leak.
Fixes: 0af5e0b41110 ("drm/meson: encoder_hdmi: switch to bridge
DRM_BRIDGE_ATTACH_NO_CONNECTOR")
Signed-off-by: Miaoqian Lin
---
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 5e306de6f485..de87f02cd388 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -435,8 +435,10 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
cec_fill_conn_info_from_drm(_info,
meson_encoder_hdmi->connector);
notifier = cec_notifier_conn_register(>dev, NULL,
_info);
- if (!notifier)
+ if (!notifier) {
+ put_device(>dev);
return -ENOMEM;
+ }
meson_encoder_hdmi->cec_notifier = notifier;
}
--
2.25.1
Re: [REPORT] syscall reboot + umh + firmware fallback
Hello, Just took a look out of curiosity. On Thu, May 12, 2022 at 02:25:57PM +0900, Byungchul Park wrote: > PROCESS A PROCESS B WORKER C > > __do_sys_reboot() > __do_sys_reboot() > mutex_lock(_transition_mutex) > ... mutex_lock(_transition_mutex) <- stuck >... > request_firmware_work_func() >_request_firmware() > firmware_fallback_sysfs() > usermodehelper_read_lock_wait() > down_read(_sem) > ... > fw_load_sysfs_fallback() > fw_sysfs_wait_timeout() > > wait_for_completion_killable_timeout(_st->completion) <- stuck > kernel_halt() > __usermodehelper_disable() >down_write(_sem) <- stuck > > > All the 3 contexts are stuck at this point. > > > PROCESS A PROCESS B WORKER C > >... >up_write(_sem) > ... > mutex_unlock(_transition_mutex) <- cannot wake up B > >... >kernel_halt() > notifier_call_chain() > hw_shutdown_notify() > kill_pending_fw_fallback_reqs() >__fw_load_abort() > complete_all(_st->completion) <- cannot wake up C > > ... > usermodeheler_read_unlock() > up_read(_sem) <- cannot wake up A I'm not sure I'm reading it correctly but it looks like "process B" column is superflous given that it's waiting on the same lock to do the same thing that A is already doing (besides, you can't really halt the machine twice). What it's reporting seems to be ABBA deadlock between A waiting on umhelper_sem and C waiting on fw_st->completion. The report seems spurious: 1. wait_for_completion_killable_timeout() doesn't need someone to wake it up to make forward progress because it will unstick itself after timeout expires. 2. complete_all() from __fw_load_abort() isn't the only source of wakeup. The fw loader can be, and mainly should be, woken up by firmware loading actually completing instead of being aborted. I guess the reason why B shows up there is because the operation order is such that just between A and C, the complete_all() takes place before __usermodehlper_disable(), so the whole thing kinda doesn't make sense as you can't block a past operation by a future one. Inserting process B introduces the reverse ordering. Thanks. -- tejun
Re: (subset) [PATCH -next] drm/vc4: hdmi: Fix build error for implicit function declaration
On Tue, 10 May 2022 21:51:48 +0800, Hui Tang wrote: > drivers/gpu/drm/vc4/vc4_hdmi.c: In function ‘vc4_hdmi_connector_detect’: > drivers/gpu/drm/vc4/vc4_hdmi.c:228:7: error: implicit declaration of function > ‘gpiod_get_value_cansleep’; did you mean ‘gpio_get_value_cansleep’? > [-Werror=implicit-function-declaration] >if (gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) >^~~~ >gpio_get_value_cansleep > CC [M] drivers/gpu/drm/vc4/vc4_validate.o > CC [M] drivers/gpu/drm/vc4/vc4_v3d.o > CC [M] drivers/gpu/drm/vc4/vc4_validate_shaders.o > CC [M] drivers/gpu/drm/vc4/vc4_debugfs.o > drivers/gpu/drm/vc4/vc4_hdmi.c: In function ‘vc4_hdmi_bind’: > drivers/gpu/drm/vc4/vc4_hdmi.c:2883:23: error: implicit declaration of > function ‘devm_gpiod_get_optional’; did you mean ‘devm_clk_get_optional’? > [-Werror=implicit-function-declaration] > vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN); >^~~ >devm_clk_get_optional > drivers/gpu/drm/vc4/vc4_hdmi.c:2883:59: error: ‘GPIOD_IN’ undeclared (first > use in this function); did you mean ‘GPIOF_IN’? > vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN); >^~~~ >GPIOF_IN > drivers/gpu/drm/vc4/vc4_hdmi.c:2883:59: note: each undeclared identifier is > reported only once for each function it appears in > cc1: all warnings being treated as errors > > [...] Applied to drm/drm-misc (drm-misc-fixes). Thanks! Maxime
Re: [PATCH 0/3] drm/client: Fix display-mode selection
On Wed, May 11, 2022 at 08:31:22PM +0200, Thomas Zimmermann wrote: > Pick user-defined display mode in DRM clients if the mode has been > validated by the driver. Otherwise pick a preferred display mode. > > Booting the kernel with video= and giving an unsupported display > mode can easily turn the display unusable. This is best tested by > booting simpledrm with a display mode that does not use the firmware > framebuffer's resolution. While simpledrm filter's out the mode as > invalid, the DRM client still picks it and the console won't show up. > > Several factors contribute to this problem. > > * The connector invalidates the user-defined display mode, but never >tells the user about it. > * The DRM client doesn't look for user-defined display modes, but for >modes that are similar. > * If no similar mode can be found, the client adds the invalid display >mode back to the connector's mode list for use. > > Each of the patches in this patchset addresses one of these problems. > Overall the DRM client has no business in display-mode detection and > should only pick one of the modes that has been detected and validated > by the connector. That's awesome, thanks! For the series, Reviewed-by: Maxime Ripard Maxime signature.asc Description: PGP signature
Re: [PATCH v2] mgag200: Enable atomic gamma lut update
Hi
Am 11.05.22 um 17:28 schrieb Jocelyn Falempe:
Add support for atomic update of gamma lut.
With this patch the "Night light" feature of gnome3
is working properly on mgag200.
v2:
- Add a default linear gamma function
- renamed functions with mgag200 prefix
- use format's 4cc code instead of bit depth
- use better interpolation for 16bits gamma
- remove legacy function mga_crtc_load_lut()
- can't remove the call to drm_mode_crtc_set_gamma_size()
because it doesn't work with userspace.
- other small refactors
Signed-off-by: Jocelyn Falempe
I already gave a Tested-by on the first iteration. It's good practice to
add these tags in follow-up patches unless the patch has changed entirely.
A few more comments are below. With those fixed:
Reviewed-by: Thomas Zimmermann
I suggest to post another version of the patch and merge it if no
further comments arrive within 2 days.
---
drivers/gpu/drm/mgag200/mgag200_mode.c | 125 -
1 file changed, 81 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 6e18d3bbd720..b748bc5b0e93 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -32,57 +32,76 @@
* This file contains setup code for the CRTC.
*/
-static void mga_crtc_load_lut(struct drm_crtc *crtc)
+static void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
+ uint32_t format)
{
- struct drm_device *dev = crtc->dev;
- struct mga_device *mdev = to_mga_device(dev);
- struct drm_framebuffer *fb;
- u16 *r_ptr, *g_ptr, *b_ptr;
int i;
- if (!crtc->enabled)
- return;
-
- if (!mdev->display_pipe.plane.state)
- return;
+ WREG8(DAC_INDEX + MGA1064_INDEX, 0);
- fb = mdev->display_pipe.plane.state->fb;
+ switch (format) {
+ case DRM_FORMAT_RGB565:
+ /* Use better interpolation, to take 32 values from 0 to 255 */
+ for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+ }
+ /* Green has one more bit, so add padding with 0 for red and
blue. */
+ for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+ }
+ break;
+ case DRM_FORMAT_RGB888:
+ case DRM_FORMAT_XRGB:
+ for (i = 0; i < MGAG200_LUT_SIZE; i++) {
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+ }
These loops look much nicer to me.
+ break;
+ default:
+ drm_warn_once(>base, "Unsupported format for gamma %d\n",
format);
There's a print format modifier for 4cc formats. It's %p4cc and expects
a pointer to the format's 4cc code. See 'git grep p4cc' for examples.
The comment itself is not easy to understand. Maybe "Unsupported format
%p4cc for gamma correction.\n" ?
+ break;
+ }
+}
- r_ptr = crtc->gamma_store;
- g_ptr = r_ptr + crtc->gamma_size;
- b_ptr = g_ptr + crtc->gamma_size;
+static void mgag200_crtc_set_gamma(struct mga_device *mdev,
+ struct drm_color_lut *lut,
+ uint32_t format)
+{
+ int i;
WREG8(DAC_INDEX + MGA1064_INDEX, 0);
- if (fb && fb->format->cpp[0] * 8 == 16) {
- int inc = (fb->format->depth == 15) ? 8 : 4;
- u8 r, b;
- for (i = 0; i < MGAG200_LUT_SIZE; i += inc) {
- if (fb->format->depth == 16) {
- if (i > (MGAG200_LUT_SIZE >> 1)) {
- r = b = 0;
- } else {
- r = *r_ptr++ >> 8;
- b = *b_ptr++ >> 8;
- r_ptr++;
- b_ptr++;
- }
- } else {
- r = *r_ptr++ >> 8;
- b = *b_ptr++ >> 8;
- }
- /* VGA registers */
- WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
+
Re: [BUG] Warning and NULL-ptr dereference in amdgpu driver with 5.18
On Tue, May 10, 2022 at 04:41:57PM -0400, Alex Deucher wrote: > Does setting amdgpu.runpm=0 on the kernel command line in grub help? > If so, that should fixed with: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f95af4a9236695caed24fe6401256bb974e8f2a7 Unfortunatly, no, this option doesn't help. Tested with v5.18-rc6, full dmesg attached. Any idea what the BadTLP messages migh be caused by? Regards, Joerg -- Jörg Rödel jroe...@suse.de SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [0.00] Linux version 5.18.0-rc6-vanilla (j...@cap.home.8bytes.org) (gcc (SUSE Linux) 11.2.1 20220420 [revision 691af15031e00227ba6d5935c1d737026cda4129], GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.38.20220411-4) #2 SMP PREEMPT_DYNAMIC Mon May 9 09:43:39 CEST 2022 [0.00] Command line: BOOT_IMAGE=/vmlinuz-5.18.0-rc6-vanilla root=/dev/mapper/cap_vg-root splash=silent resume=/dev/cap_vg/swap mitigations=auto quiet iommu=nopt amdgpu.runpm=0 [0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers' [0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers' [0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers' [0.00] x86/fpu: xstate_offset[2]: 576, xstate_sizes[2]: 256 [0.00] x86/fpu: Enabled xstate features 0x7, context size is 832 bytes, using 'compacted' format. [0.00] signal: max sigframe size: 1776 [0.00] BIOS-provided physical RAM map: [0.00] BIOS-e820: [mem 0x-0x0009] usable [0.00] BIOS-e820: [mem 0x000a-0x000f] reserved [0.00] BIOS-e820: [mem 0x0010-0x09cf] usable [0.00] BIOS-e820: [mem 0x09d0-0x09ff] reserved [0.00] BIOS-e820: [mem 0x0a00-0x0a1f] usable [0.00] BIOS-e820: [mem 0x0a20-0x0a20afff] ACPI NVS [0.00] BIOS-e820: [mem 0x0a20b000-0x0aff] usable [0.00] BIOS-e820: [mem 0x0b00-0x0b01] reserved [0.00] BIOS-e820: [mem 0x0b02-0xd0214fff] usable [0.00] BIOS-e820: [mem 0xd0215000-0xd0236fff] ACPI data [0.00] BIOS-e820: [mem 0xd0237000-0xd9103fff] usable [0.00] BIOS-e820: [mem 0xd9104000-0xd9273fff] reserved [0.00] BIOS-e820: [mem 0xd9274000-0xd9282fff] ACPI data [0.00] BIOS-e820: [mem 0xd9283000-0xd939bfff] usable [0.00] BIOS-e820: [mem 0xd939c000-0xd9790fff] ACPI NVS [0.00] BIOS-e820: [mem 0xd9791000-0xda58cfff] reserved [0.00] BIOS-e820: [mem 0xda58d000-0xdcff] usable [0.00] BIOS-e820: [mem 0xdd00-0xdfff] reserved [0.00] BIOS-e820: [mem 0xf800-0xfbff] reserved [0.00] BIOS-e820: [mem 0xfd10-0xfd1f] reserved [0.00] BIOS-e820: [mem 0xfea0-0xfea0] reserved [0.00] BIOS-e820: [mem 0xfeb8-0xfec01fff] reserved [0.00] BIOS-e820: [mem 0xfec1-0xfec10fff] reserved [0.00] BIOS-e820: [mem 0xfec3-0xfec30fff] reserved [0.00] BIOS-e820: [mem 0xfed0-0xfed00fff] reserved [0.00] BIOS-e820: [mem 0xfed4-0xfed44fff] reserved [0.00] BIOS-e820: [mem 0xfed8-0xfed8] reserved [0.00] BIOS-e820: [mem 0xfedc2000-0xfedc] reserved [0.00] BIOS-e820: [mem 0xfedd4000-0xfedd5fff] reserved [0.00] BIOS-e820: [mem 0xfee0-0xfeef] reserved [0.00] BIOS-e820: [mem 0xff00-0x] reserved [0.00] BIOS-e820: [mem 0x0001-0x00101f37] usable [0.00] NX (Execute Disable) protection: active [0.00] e820: update [mem 0xcfe9b018-0xcfedca57] usable ==> usable [0.00] e820: update [mem 0xcfe9b018-0xcfedca57] usable ==> usable [0.00] e820: update [mem 0xcfe59018-0xcfe9aa57] usable ==> usable [0.00] e820: update [mem 0xcfe59018-0xcfe9aa57] usable ==> usable [0.00] e820: update [mem 0xcfe47018-0xcfe58057] usable ==> usable [0.00] e820: update [mem 0xcfe47018-0xcfe58057] usable ==> usable [0.00] e820: update [mem 0xcfe2a018-0xcfe46c57] usable ==> usable [0.00] e820: update [mem 0xcfe2a018-0xcfe46c57] usable ==> usable [0.00] extended physical RAM map: [0.00] reserve setup_data: [mem 0x-0x0009] usable [0.00] reserve setup_data: [mem 0x000a-0x000f] reserved [0.00] reserve setup_data:
Re: [PATCH] drm/atomic-helpers: remove legacy_cursor_update hacks
Hi Daniel, An update on this On Thu, Apr 28, 2022 at 02:09:40PM +0200, Daniel Vetter wrote: > > We integrated this in the (downstream) RaspberryPi kernel, and it seems > > to trigger some weird regressions: > > > > - If we move the cursor under X, the primary plane update is stuck: > > https://github.com/raspberrypi/linux/issues/4988 So it turns out the upstream driver doesn't seem affected by this, but only a downstream alternative. > > - Switching back and forth between VT gets the kernel stuck (with a > > locking issue in fb_release?) > > https://github.com/raspberrypi/linux/issues/5011 And this one turned out to be a separate issue fixed by Javier already. So as far as I'm concerned, this patch seems to be working fine on vc4 Maxime signature.asc Description: PGP signature
Re: (subset) [PATCH] MAINTAINERS: add Melissa to V3D maintainers
On Fri, 29 Apr 2022 18:33:30 -0100, Melissa Wen wrote: > I've been contributing to v3d through improvements, reviews, testing, > debugging, etc. So, I'm adding myself as a co-maintainer of the V3D > driver. > > Applied to drm/drm-misc (drm-misc-next). Thanks! Maxime
Re: [PATCH v9 18/22] drm/mediatek: Add mt8195 Embedded DisplayPort driver
Hi,
On Wed, May 11, 2022 at 05:59:13AM -0700, Guillaume Ranquet wrote:
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#include "mtk_dp_reg.h"
> >> +
> >> +#define MTK_DP_AUX_WAIT_REPLY_COUNT 20
> >> +#define MTK_DP_CHECK_SINK_CAP_TIMEOUT_COUNT 3
> >> +
> >> +//TODO: platform/device data or dts?
> >
> >DTS :)
>
> It's probably going to be a platform_data struct for v10...
> If I have time, I'll change it to a dts property for v10.
I can't really imagine a case where we would need platform_data
nowadays. If you have a device tree, then it should be part of the
binding.
What issue would you like to address by using a platform_data?
> >> +static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge
> >> *bridge)
> >> +{
> >> + return connector_status_connected;
> >> +}
> >
> >I'm not quite sure what's going on there. You seem to have some support
> >for HPD interrupts above, but you always report the display as
> >connected?
> >
> >I'd assume that either you don't have HPD support and then always report
> >it as connected, or you have HPD support and report the current status
> >in detect, but that combination seems weird.
>
> The HPD logic needs more work, some things have been broken when I split
> the driver into three patches eDP - DP - Audio
> The assumption at first was that eDP didn't need any HPD handling... but it
> seems I was wrong and the eDP driver needs to be reworked.
That can be made into a patch of its own if you prefer.
You first introduce the driver without status reporting (always
returning connected or unknown), and then add the needed bits for HPD.
However, that first patch shouldn't contain the interrupt plumbing and
so on, it's just confusing.
> >> +static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge,
> >> + struct drm_connector *connector)
> >> +{
> >> + struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
> >> + bool enabled = mtk_dp->enabled;
> >> + struct edid *new_edid = NULL;
> >> +
> >> + if (!enabled)
> >> + drm_bridge_chain_pre_enable(bridge);
> >> +
> >> + drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
> >> + usleep_range(2000, 5000);
> >> +
> >> + if (mtk_dp_plug_state(mtk_dp))
> >> + new_edid = drm_get_edid(connector, _dp->aux.ddc);
> >> +
> >> + if (!enabled)
> >> + drm_bridge_chain_post_disable(bridge);
> >
> >Are you sure we can't get a mode set while get_edid is called?
> >
> >If we can, then you could end up disabling the device while it's being
> >powered on.
>
> I'm a bit unsure, I need to spend more time in the drm stack to make sure.
> I'll get back to you when I have a definitive answer.
So, it looks like it's ok.
get_edid is your implementation of get_modes, which is called by
drm_helper_probe_single_connector_modes
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_probe_helper.c#L416
This is the standard implemantion of fill_modes, which is called
whenever the get_connector ioctl is called (or similar paths, like
drm_client_modeset_probe)
drm_helper_probe_single_connector_modes is under the assumption that the
mode_config.mutex is held though, and that the big lock. So it should be
serialized there.
Just for future proofing though, it would be better to use refcounting
there. Would runtime_pm work for you there?
> >> +static void mtk_dp_parse_drm_mode_timings(struct mtk_dp *mtk_dp,
> >> +struct drm_display_mode *mode)
> >> +{
> >> + struct mtk_dp_timings *timings = _dp->info.timings;
> >> +
> >> + drm_display_mode_to_videomode(mode, >vm);
> >> + timings->frame_rate = mode->clock * 1000 / mode->htotal / mode->vtotal;
> >
> >drm_mode_vrefresh()
> >
> >> + timings->htotal = mode->htotal;
> >> + timings->vtotal = mode->vtotal;
> >> +}
> >
> >It's not really clear to me why you need to duplicate drm_display_mode
> >here?
> >
> It's saved to be re-used in mtk_dp_set_msa().
> It's not ideal, I'll check if I can get the mode directly from
> mtk_dp_set_msa()
Yeah, it looks like mtk_dp_set_msa() uses fairly straightforward values,
this will be just as easy with drm_display_mode.
Maxime
Re: [PATCH v4 10/15] drm/shmem-helper: Take reservation lock instead of drm_gem_shmem locks
Am 11.05.22 um 21:05 schrieb Daniel Vetter: [SNIP] It's unclear to me which driver may ever want to do the mapping under the dma_resv_lock. But if we will ever have such a driver that will need to map imported buffer under dma_resv_lock, then we could always add the dma_buf_vmap_locked() variant of the function. In this case the locking rule will sound like this: "All dma-buf importers are responsible for holding the dma-reservation lock around the dmabuf->ops->mmap/vmap() calls." Are you okay with this rule? Yeah I think long-term it's where we want to be, just trying to find clever ways to get there. And I think Christian agrees with that? Yes, completely. A design where most DMA-buf functions are supposed to be called with the reservation lock held is exactly what I have in mind for the long term. It shouldn't be that hard to clean up. The last time I looked into it my main problem was that we didn't had any easy unit test for it. Do we have any tests for dma-bufs at all? It's unclear to me what you are going to test in regards to the reservation locks, could you please clarify? Unfortunately not really :-/ Only way really is to grab a driver which needs vmap (those are mostly display drivers) on an imported buffer, and see what happens. 2nd best is liberally sprinkling lockdep annotations all over the place and throwing it at intel ci (not sure amd ci is accessible to the public) and then hoping that's good enough. Stuff like might_lock and dma_resv_assert_held. Alright So throwing it at intel-gfx-ci can't hurt I think, but that only covers i915 so doesn't really help with the bigger issue of catching all the drivers. BTW: We have now somebody working on converting the existing libdrm_amdgpu unit tests over to igt. Regards, Christian. Cheers, Daniel
Re: [PATCH 3/3] drm/client: Don't add new command-line mode
On 5/11/22 20:31, Thomas Zimmermann wrote: > Don't add a mode for the kernel's command-line parameters from > within the DRM client code. Doing so can result in an unusable > display. If there's no compatible command-line mode, the client > will use one of the connector's preferred modes. > > All mode creation and validation has to be performed by the > connector. When clients run, the connector's fill_modes callback > has already processes the kernel parameters and validates each s/process/processed and s/validates/validated ? or remove the "has" in the sentence, either work I think. > mode before adding it. The connector's mode list does not contain > invalid modes. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/drm_client_modeset.c | 17 + > 1 file changed, 5 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/drm_client_modeset.c > b/drivers/gpu/drm/drm_client_modeset.c > index b777faa87f04..48e6ce16439f 100644 > --- a/drivers/gpu/drm/drm_client_modeset.c > +++ b/drivers/gpu/drm/drm_client_modeset.c > @@ -158,8 +158,7 @@ drm_connector_has_preferred_mode(struct drm_connector > *connector, int width, int > return NULL; > } > > -static struct drm_display_mode * > -drm_connector_pick_cmdline_mode(struct drm_connector *connector) > +static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct > drm_connector *connector) This seems like an unrelated cleanup. The rest looks good to me. Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
[PATCH] drm/amd/display: clean up some inconsistent indenting
Eliminate the follow smatch warning: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9687 amdgpu_dm_atomic_commit_tail() warn: inconsistent indenting. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index e2b57cf6506c..a92cfb055c15 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9681,9 +9681,10 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) dm_enable_per_frame_crtc_master_sync(dc_state); mutex_lock(>dc_lock); WARN_ON(!dc_commit_state(dm->dc, dc_state)); - /* Allow idle optimization when vblank count is 0 for display off */ - if (dm->active_vblank_irq_count == 0) - dc_allow_idle_optimizations(dm->dc,true); + + /* Allow idle optimization when vblank count is 0 for display off */ + if (dm->active_vblank_irq_count == 0) + dc_allow_idle_optimizations(dm->dc, true); mutex_unlock(>dc_lock); } -- 2.20.1.7.g153144c
Re: [PATCH 2/3] drm/client: Look for command-line modes first
On 5/11/22 20:31, Thomas Zimmermann wrote: > When picking a mode, first look for modes that have been specified > by the user on the kernel's command line. Only if that fails, use > the existing heuristic of picking a nearby mode from it's various > parameters. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
