date:20230906

Add Cadence HDP-TX HDMI PHY driver for i.MX8MQ.

Cadence HDP-TX PHY could be put in either DP mode or
HDMI mode base on the configuration chosen.
HDMI PHY mode is configurated in the driver.

Signed-off-by: Sandor Yu 
Tested-by: Alexander Stein 
---
 drivers/phy/freescale/Kconfig   |   9 +
 drivers/phy/freescale/Makefile  |   1 +
 drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c | 955 
 3 files changed, 965 insertions(+)
 create mode 100644 drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c

diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index 2999ba1e57d0..0c07fccba917 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -44,6 +44,15 @@ config PHY_FSL_IMX8MQ_DP_PHY
  Enable this to support the Cadence HDPTX DP PHY driver
  on i.MX8MQ SOC.
 
+config PHY_FSL_IMX8MQ_HDMI_PHY
+   tristate "Freescale i.MX8MQ HDMI PHY support"
+   depends on OF && HAS_IOMEM
+   depends on COMMON_CLK
+   select GENERIC_PHY
+   help
+ Enable this to support the Cadence HDPTX HDMI PHY driver
+ on i.MX8MQ SOC.
+
 endif
 
 config PHY_FSL_LYNX_28G
diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
index 915a429d9fbc..245783c04951 100644
--- a/drivers/phy/freescale/Makefile
+++ b/drivers/phy/freescale/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)   += 
phy-fsl-imx8-mipi-dphy.o
 obj-$(CONFIG_PHY_FSL_IMX8M_PCIE)   += phy-fsl-imx8m-pcie.o
 obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o
 obj-$(CONFIG_PHY_FSL_IMX8MQ_DP_PHY)+= phy-fsl-imx8mq-dp.o
+obj-$(CONFIG_PHY_FSL_IMX8MQ_HDMI_PHY)  += phy-fsl-imx8mq-hdmi.o
diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c 
b/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
new file mode 100644
index ..fffaaa888ba2
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-hdmi.c
@@ -0,0 +1,955 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Cadence High-Definition Multimedia Interface (HDMI) PHY driver
+ *
+ * Copyright (C) 2022,2023 NXP Semiconductor, Inc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define ADDR_PHY_AFE   0x8
+
+/* PHY registers */
+#define CMN_SSM_BIAS_TMR   0x0022
+#define CMN_PLLSM0_USER_DEF_CTRL   0x002f
+#define CMN_PSM_CLK_CTRL   0x0061
+#define CMN_CDIAG_REFCLK_CTRL  0x0062
+#define CMN_PLL0_VCOCAL_START  0x0081
+#define CMN_PLL0_VCOCAL_INIT_TMR   0x0084
+#define CMN_PLL0_VCOCAL_ITER_TMR   0x0085
+#define CMN_TXPUCAL_CTRL   0x00e0
+#define CMN_TXPDCAL_CTRL   0x00f0
+#define CMN_TXPU_ADJ_CTRL  0x0108
+#define CMN_TXPD_ADJ_CTRL  0x010c
+#define CMN_DIAG_PLL0_FBH_OVRD 0x01c0
+#define CMN_DIAG_PLL0_FBL_OVRD 0x01c1
+#define CMN_DIAG_PLL0_OVRD 0x01c2
+#define CMN_DIAG_PLL0_TEST_MODE0x01c4
+#define CMN_DIAG_PLL0_V2I_TUNE 0x01c5
+#define CMN_DIAG_PLL0_CP_TUNE  0x01c6
+#define CMN_DIAG_PLL0_LF_PROG  0x01c7
+#define CMN_DIAG_PLL0_PTATIS_TUNE1 0x01c8
+#define CMN_DIAG_PLL0_PTATIS_TUNE2 0x01c9
+#define CMN_DIAG_PLL0_INCLK_CTRL   0x01ca
+#define CMN_DIAG_PLL0_PXL_DIVH 0x01cb
+#define CMN_DIAG_PLL0_PXL_DIVL 0x01cc
+#define CMN_DIAG_HSCLK_SEL 0x01e0
+#define XCVR_PSM_RCTRL 0x4001
+#define TX_TXCC_CAL_SCLR_MULT_00x4047
+#define TX_TXCC_CPOST_MULT_00_00x404c
+#define XCVR_DIAG_PLLDRC_CTRL  0x40e0
+#define XCVR_DIAG_PLLDRC_CTRL  0x40e0
+#define XCVR_DIAG_HSCLK_SEL0x40e1
+#define XCVR_DIAG_BIDI_CTRL0x40e8
+#define TX_PSC_A0  0x4100
+#define TX_PSC_A1  0x4101
+#define TX_PSC_A2  0x4102
+#define TX_PSC_A3  0x4103
+#define TX_DIAG_TX_CTRL0x41e0
+#define TX_DIAG_TX_DRV 0x41e1
+#define TX_DIAG_BGREF_PREDRV_DELAY 0x41e7
+#define TX_DIAG_ACYA_0 0x41ff
+#define TX_DIAG_ACYA_1 0x43ff
+#define TX_DIAG_ACYA_2 0x45ff
+#define TX_DIAG_ACYA_3 0x47ff
+#define TX_ANA_CTRL_REG_1  0x5020
+#define TX_ANA_CTRL_REG_2  0x5021
+#define TX_DIG_CTRL_REG_2  0x5024
+#define TXDA_CYA_AUXDA_CYA 0x5025
+#define TX_ANA_CTRL_REG_3  0x5026
+#define TX_ANA_CTRL_REG_4  0x5027
+#define TX_ANA_CTRL_REG_5  0x5029
+#define

[PATCH v9 6/7] phy: freescale: Add DisplayPort PHY driver for i.MX8MQ

Add Cadence HDP-TX DisplayPort PHY driver for i.MX8MQ

Cadence HDP-TX PHY could be put in either DP mode or
HDMI mode base on the configuration chosen.
DisplayPort PHY mode is configurated in the driver.

Signed-off-by: Sandor Yu 
---
 drivers/phy/freescale/Kconfig |   9 +
 drivers/phy/freescale/Makefile|   1 +
 drivers/phy/freescale/phy-fsl-imx8mq-dp.c | 714 ++
 3 files changed, 724 insertions(+)
 create mode 100644 drivers/phy/freescale/phy-fsl-imx8mq-dp.c

diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
index 853958fb2c06..2999ba1e57d0 100644
--- a/drivers/phy/freescale/Kconfig
+++ b/drivers/phy/freescale/Kconfig
@@ -35,6 +35,15 @@ config PHY_FSL_IMX8M_PCIE
  Enable this to add support for the PCIE PHY as found on
  i.MX8M family of SOCs.
 
+config PHY_FSL_IMX8MQ_DP_PHY
+   tristate "Freescale i.MX8MQ DP PHY support"
+   depends on OF && HAS_IOMEM
+   depends on COMMON_CLK
+   select GENERIC_PHY
+   help
+ Enable this to support the Cadence HDPTX DP PHY driver
+ on i.MX8MQ SOC.
+
 endif
 
 config PHY_FSL_LYNX_28G
diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile
index cedb328bc4d2..915a429d9fbc 100644
--- a/drivers/phy/freescale/Makefile
+++ b/drivers/phy/freescale/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_PHY_MIXEL_LVDS_PHY)+= 
phy-fsl-imx8qm-lvds-phy.o
 obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)  += phy-fsl-imx8-mipi-dphy.o
 obj-$(CONFIG_PHY_FSL_IMX8M_PCIE)   += phy-fsl-imx8m-pcie.o
 obj-$(CONFIG_PHY_FSL_LYNX_28G) += phy-fsl-lynx-28g.o
+obj-$(CONFIG_PHY_FSL_IMX8MQ_DP_PHY)+= phy-fsl-imx8mq-dp.o
diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-dp.c 
b/drivers/phy/freescale/phy-fsl-imx8mq-dp.c
new file mode 100644
index ..b1f45c0b27b5
--- /dev/null
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-dp.c
@@ -0,0 +1,714 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Cadence HDP-TX Display Port Interface (DP) PHY driver
+ *
+ * Copyright (C) 2022, 2023 NXP Semiconductor, Inc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define ADDR_PHY_AFE   0x8
+
+/* PHY registers */
+#define CMN_SSM_BIAS_TMR   0x0022
+#define CMN_PLLSM0_PLLEN_TMR   0x0029
+#define CMN_PLLSM0_PLLPRE_TMR  0x002a
+#define CMN_PLLSM0_PLLVREF_TMR 0x002b
+#define CMN_PLLSM0_PLLLOCK_TMR 0x002c
+#define CMN_PLLSM0_USER_DEF_CTRL   0x002f
+#define CMN_PSM_CLK_CTRL   0x0061
+#define CMN_PLL0_VCOCAL_START  0x0081
+#define CMN_PLL0_VCOCAL_INIT_TMR   0x0084
+#define CMN_PLL0_VCOCAL_ITER_TMR   0x0085
+#define CMN_PLL0_INTDIV0x0094
+#define CMN_PLL0_FRACDIV   0x0095
+#define CMN_PLL0_HIGH_THR  0x0096
+#define CMN_PLL0_DSM_DIAG  0x0097
+#define CMN_PLL0_SS_CTRL2  0x0099
+#define CMN_ICAL_INIT_TMR  0x00c4
+#define CMN_ICAL_ITER_TMR  0x00c5
+#define CMN_RXCAL_INIT_TMR 0x00d4
+#define CMN_RXCAL_ITER_TMR 0x00d5
+#define CMN_TXPUCAL_INIT_TMR   0x00e4
+#define CMN_TXPUCAL_ITER_TMR   0x00e5
+#define CMN_TXPDCAL_INIT_TMR   0x00f4
+#define CMN_TXPDCAL_ITER_TMR   0x00f5
+#define CMN_ICAL_ADJ_INIT_TMR  0x0102
+#define CMN_ICAL_ADJ_ITER_TMR  0x0103
+#define CMN_RX_ADJ_INIT_TMR0x0106
+#define CMN_RX_ADJ_ITER_TMR0x0107
+#define CMN_TXPU_ADJ_INIT_TMR  0x010a
+#define CMN_TXPU_ADJ_ITER_TMR  0x010b
+#define CMN_TXPD_ADJ_INIT_TMR  0x010e
+#define CMN_TXPD_ADJ_ITER_TMR  0x010f
+#define CMN_DIAG_PLL0_FBH_OVRD 0x01c0
+#define CMN_DIAG_PLL0_FBL_OVRD 0x01c1
+#define CMN_DIAG_PLL0_OVRD 0x01c2
+#define CMN_DIAG_PLL0_TEST_MODE0x01c4
+#define CMN_DIAG_PLL0_V2I_TUNE 0x01c5
+#define CMN_DIAG_PLL0_CP_TUNE  0x01c6
+#define CMN_DIAG_PLL0_LF_PROG  0x01c7
+#define CMN_DIAG_PLL0_PTATIS_TUNE1 0x01c8
+#define CMN_DIAG_PLL0_PTATIS_TUNE2 0x01c9
+#define CMN_DIAG_HSCLK_SEL 0x01e0
+#define CMN_DIAG_PER_CAL_ADJ   0x01ec
+#define CMN_DIAG_CAL_CTRL  0x01ed
+#define CMN_DIAG_ACYA  0x01ff
+#define XCVR_PSM_RCTRL 0x4001
+#define XCVR_PSM_CAL_TMR   0x4002
+#define XCVR_PSM_A0IN_TMR  0x4003
+#define TX_TXCC_CAL_SCLR_MULT_00x4047
+#define TX_TXCC_CPOST_MULT_00_00x404c
+#define XCVR_DIAG_PLLDRC_CTRL

[PATCH v9 5/7] dt-bindings: phy: Add Freescale iMX8MQ DP and HDMI PHY

Add bindings for Freescale iMX8MQ DP and HDMI PHY.

Signed-off-by: Sandor Yu 
Reviewed-by: Rob Herring 
---
 .../bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml  | 53 +++
 1 file changed, 53 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml 
b/Documentation/devicetree/bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml
new file mode 100644
index ..917f113503dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/fsl,imx8mq-dp-hdmi-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cadence HDP-TX DP/HDMI PHY for Freescale i.MX8MQ SoC
+
+maintainers:
+  - Sandor Yu 
+
+properties:
+  compatible:
+enum:
+  - fsl,imx8mq-dp-phy
+  - fsl,imx8mq-hdmi-phy
+
+  reg:
+maxItems: 1
+
+  clocks:
+items:
+  - description: PHY reference clock.
+  - description: APB clock.
+
+  clock-names:
+items:
+  - const: ref
+  - const: apb
+
+  "#phy-cells":
+const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+#include 
+dp_phy: phy@32c0 {
+compatible = "fsl,imx8mq-dp-phy";
+reg = <0x32c0 0x10>;
+#phy-cells = <0>;
+clocks = <_phy_27m>, < IMX8MQ_CLK_DISP_APB_ROOT>;
+clock-names = "ref", "apb";
+};
-- 
2.34.1

[PATCH v9 4/7] drm: bridge: Cadence: Add MHDP8501 DP/HDMI driver

Add a new DRM DisplayPort and HDMI bridge driver for Candence MHDP8501
used in i.MX8MQ SOC. MHDP8501 could support HDMI or DisplayPort
standards according embedded Firmware running in the uCPU.

For iMX8MQ SOC, the DisplayPort/HDMI FW was loaded and activated by
SOC's ROM code. Bootload binary included respective specific firmware
is required.

Driver will check display connector type and
then load the corresponding driver.

Signed-off-by: Sandor Yu 
Tested-by: Alexander Stein 
---
v8->v9:
* Remove compatible string "cdns,mhdp8501" that had removed
  from dt-bindings file in v8.

 drivers/gpu/drm/bridge/cadence/Kconfig|  15 +
 drivers/gpu/drm/bridge/cadence/Makefile   |   2 +
 .../drm/bridge/cadence/cdns-mhdp8501-core.c   | 312 +++
 .../drm/bridge/cadence/cdns-mhdp8501-core.h   | 410 +
 .../gpu/drm/bridge/cadence/cdns-mhdp8501-dp.c | 780 ++
 .../drm/bridge/cadence/cdns-mhdp8501-hdmi.c   | 674 +++
 6 files changed, 2193 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.c
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.h
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-dp.c
 create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c

diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig 
b/drivers/gpu/drm/bridge/cadence/Kconfig
index ec35215a2003..d9daf7ec0cd5 100644
--- a/drivers/gpu/drm/bridge/cadence/Kconfig
+++ b/drivers/gpu/drm/bridge/cadence/Kconfig
@@ -46,3 +46,18 @@ config DRM_CDNS_MHDP8546_J721E
  initializes the J721E Display Port and sets up the
  clock and data muxes.
 endif
+
+config DRM_CDNS_MHDP8501
+   tristate "Cadence MHDP8501 DP/HDMI bridge"
+   select DRM_KMS_HELPER
+   select DRM_PANEL_BRIDGE
+   select DRM_DISPLAY_DP_HELPER
+   select DRM_DISPLAY_HELPER
+   select DRM_CDNS_AUDIO
+   depends on OF
+   help
+ Support Cadence MHDP8501 DisplayPort/HDMI bridge.
+ Cadence MHDP8501 support one or more protocols,
+ including DisplayPort and HDMI.
+ To use the DP and HDMI drivers, their respective
+ specific firmware is required.
diff --git a/drivers/gpu/drm/bridge/cadence/Makefile 
b/drivers/gpu/drm/bridge/cadence/Makefile
index c95fd5b81d13..ea327287d1c1 100644
--- a/drivers/gpu/drm/bridge/cadence/Makefile
+++ b/drivers/gpu/drm/bridge/cadence/Makefile
@@ -5,3 +5,5 @@ cdns-dsi-$(CONFIG_DRM_CDNS_DSI_J721E) += cdns-dsi-j721e.o
 obj-$(CONFIG_DRM_CDNS_MHDP8546) += cdns-mhdp8546.o
 cdns-mhdp8546-y := cdns-mhdp8546-core.o cdns-mhdp8546-hdcp.o
 cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += cdns-mhdp8546-j721e.o
+obj-$(CONFIG_DRM_CDNS_MHDP8501) += cdns-mhdp8501.o
+cdns-mhdp8501-y := cdns-mhdp8501-core.o cdns-mhdp8501-dp.o cdns-mhdp8501-hdmi.o
diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.c 
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.c
new file mode 100644
index ..f885679967d6
--- /dev/null
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-core.c
@@ -0,0 +1,312 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Cadence Display Port Interface (DP) driver
+ *
+ * Copyright (C) 2023 NXP Semiconductor, Inc.
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "cdns-mhdp8501-core.h"
+
+static int cdns_mhdp8501_read_hpd(struct cdns_mhdp_device *mhdp)
+{
+   u8 status;
+   int ret;
+
+   mutex_lock(>mbox_mutex);
+
+   ret = cdns_mhdp_mailbox_send(mhdp, MB_MODULE_ID_GENERAL,
+GENERAL_GET_HPD_STATE, 0, NULL);
+   if (ret)
+   goto err_get_hpd;
+
+   ret = cdns_mhdp_mailbox_recv_header(mhdp, MB_MODULE_ID_GENERAL,
+   GENERAL_GET_HPD_STATE,
+   sizeof(status));
+   if (ret)
+   goto err_get_hpd;
+
+   ret = cdns_mhdp_mailbox_recv_data(mhdp, , sizeof(status));
+   if (ret)
+   goto err_get_hpd;
+
+   mutex_unlock(>mbox_mutex);
+
+   return status;
+
+err_get_hpd:
+   DRM_ERROR("read hpd  failed: %d\n", ret);
+   mutex_unlock(>mbox_mutex);
+
+   return ret;
+}
+
+enum drm_connector_status cdns_mhdp8501_detect(struct cdns_mhdp_device *mhdp)
+{
+   u8 hpd = 0xf;
+
+   hpd = cdns_mhdp8501_read_hpd(mhdp);
+   if (hpd == 1)
+   return connector_status_connected;
+   else if (hpd == 0)
+   return connector_status_disconnected;
+
+   DRM_INFO("Unknown cable status, hdp=%u\n", hpd);
+   return connector_status_unknown;
+}
+
+static void hotplug_work_func(struct work_struct *work)
+{
+   struct cdns_mhdp_device *mhdp = container_of(work,
+struct cdns_mhdp_device,
+hotplug_work.work);
+   enum drm_connector_status status =

[PATCH v9 3/7] dt-bindings: display: bridge: Add Cadence MHDP850

Add bindings for Cadence MHDP8501 DisplayPort/HDMI bridge.

Signed-off-by: Sandor Yu 
Reviewed-by: Krzysztof Kozlowski 
---
v8->v9:
* Add Krzysztof's R-b tag

 .../display/bridge/cdns,mhdp8501.yaml | 104 ++
 1 file changed, 104 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml

diff --git 
a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml 
b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml
new file mode 100644
index ..3ae643845cfe
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/cdns,mhdp8501.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cadence MHDP8501 DP/HDMI bridge
+
+maintainers:
+  - Sandor Yu 
+
+description:
+  Cadence MHDP8501 DisplayPort/HDMI interface.
+
+properties:
+  compatible:
+enum:
+  - fsl,imx8mq-mhdp8501
+
+  reg:
+maxItems: 1
+
+  clocks:
+maxItems: 1
+description: MHDP8501 DP/HDMI APB clock.
+
+  phys:
+maxItems: 1
+description:
+  phandle to the DisplayPort or HDMI PHY
+
+  interrupts:
+items:
+  - description: Hotplug cable plugin.
+  - description: Hotplug cable plugout.
+
+  interrupt-names:
+items:
+  - const: plug_in
+  - const: plug_out
+
+  ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+  port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+  Input port from display controller output.
+  port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description:
+  Output port to DisplayPort or HDMI connector.
+
+required:
+  - port@0
+  - port@1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+  - interrupt-names
+  - phys
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+#include 
+
+mhdp: display-bridge@32c0 {
+compatible = "fsl,imx8mq-mhdp8501";
+reg = <0x32c0 0x10>;
+interrupts = ,
+ ;
+interrupt-names = "plug_in", "plug_out";
+clocks = < IMX8MQ_CLK_DISP_APB_ROOT>;
+phys = <_phy>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+
+mhdp_in: endpoint {
+remote-endpoint = <_out>;
+};
+};
+
+port@1 {
+reg = <1>;
+
+mhdp_out: endpoint {
+remote-endpoint = <_connector>;
+};
+};
+};
+};
-- 
2.34.1

[PATCH v9 2/7] phy: Add HDMI configuration options

Allow HDMI PHYs to be configured through the generic
functions through a custom structure added to the generic union.

The parameters added here are based on HDMI PHY
implementation practices.  The current set of parameters
should cover the potential users.

Signed-off-by: Sandor Yu 
Reviewed-by: Dmitry Baryshkov 
---
v8->v9:
* Add Dmitry's R-b tag

 include/linux/phy/phy-hdmi.h | 24 
 include/linux/phy/phy.h  |  7 ++-
 2 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/phy/phy-hdmi.h

diff --git a/include/linux/phy/phy-hdmi.h b/include/linux/phy/phy-hdmi.h
new file mode 100644
index ..b7de88e9090f
--- /dev/null
+++ b/include/linux/phy/phy-hdmi.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2022 NXP
+ */
+
+#ifndef __PHY_HDMI_H_
+#define __PHY_HDMI_H_
+
+#include 
+/**
+ * struct phy_configure_opts_hdmi - HDMI configuration set
+ * @pixel_clk_rate: Pixel clock of video modes in KHz.
+ * @bpc: Maximum bits per color channel.
+ * @color_space: Colorspace in enum hdmi_colorspace.
+ *
+ * This structure is used to represent the configuration state of a HDMI phy.
+ */
+struct phy_configure_opts_hdmi {
+   unsigned int pixel_clk_rate;
+   unsigned int bpc;
+   enum hdmi_colorspace color_space;
+};
+
+#endif /* __PHY_HDMI_H_ */
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index f6d607ef0e80..94d489a8a163 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -17,6 +17,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 
@@ -42,7 +43,8 @@ enum phy_mode {
PHY_MODE_MIPI_DPHY,
PHY_MODE_SATA,
PHY_MODE_LVDS,
-   PHY_MODE_DP
+   PHY_MODE_DP,
+   PHY_MODE_HDMI,
 };
 
 enum phy_media {
@@ -60,11 +62,14 @@ enum phy_media {
  * the DisplayPort protocol.
  * @lvds:  Configuration set applicable for phys supporting
  * the LVDS phy mode.
+ * @hdmi:  Configuration set applicable for phys supporting
+ * the HDMI phy mode.
  */
 union phy_configure_opts {
struct phy_configure_opts_mipi_dphy mipi_dphy;
struct phy_configure_opts_dpdp;
struct phy_configure_opts_lvds  lvds;
+   struct phy_configure_opts_hdmi  hdmi;
 };
 
 /**
-- 
2.34.1

[PATCH v9 1/7] drm: bridge: Cadence: convert mailbox functions to macro functions

MHDP8546 mailbox access functions will be share to other mhdp driver
and Cadence HDP-TX HDMI/DP PHY drivers.
Move those functions to head file include/drm/bridge/cdns-mhdp-mailbox.h
and convert them to macro functions.

Signed-off-by: Sandor Yu 
---
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 195 +-
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |   1 -
 include/drm/bridge/cdns-mhdp-mailbox.h| 240 ++
 3 files changed, 241 insertions(+), 195 deletions(-)
 create mode 100644 include/drm/bridge/cdns-mhdp-mailbox.h

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c 
b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index f6822dfa3805..ddd3c633c7bf 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -54,200 +55,6 @@
 #include "cdns-mhdp8546-hdcp.h"
 #include "cdns-mhdp8546-j721e.h"
 
-static int cdns_mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
-{
-   int ret, empty;
-
-   WARN_ON(!mutex_is_locked(>mbox_mutex));
-
-   ret = readx_poll_timeout(readl, mhdp->regs + CDNS_MAILBOX_EMPTY,
-empty, !empty, MAILBOX_RETRY_US,
-MAILBOX_TIMEOUT_US);
-   if (ret < 0)
-   return ret;
-
-   return readl(mhdp->regs + CDNS_MAILBOX_RX_DATA) & 0xff;
-}
-
-static int cdns_mhdp_mailbox_write(struct cdns_mhdp_device *mhdp, u8 val)
-{
-   int ret, full;
-
-   WARN_ON(!mutex_is_locked(>mbox_mutex));
-
-   ret = readx_poll_timeout(readl, mhdp->regs + CDNS_MAILBOX_FULL,
-full, !full, MAILBOX_RETRY_US,
-MAILBOX_TIMEOUT_US);
-   if (ret < 0)
-   return ret;
-
-   writel(val, mhdp->regs + CDNS_MAILBOX_TX_DATA);
-
-   return 0;
-}
-
-static int cdns_mhdp_mailbox_recv_header(struct cdns_mhdp_device *mhdp,
-u8 module_id, u8 opcode,
-u16 req_size)
-{
-   u32 mbox_size, i;
-   u8 header[4];
-   int ret;
-
-   /* read the header of the message */
-   for (i = 0; i < sizeof(header); i++) {
-   ret = cdns_mhdp_mailbox_read(mhdp);
-   if (ret < 0)
-   return ret;
-
-   header[i] = ret;
-   }
-
-   mbox_size = get_unaligned_be16(header + 2);
-
-   if (opcode != header[0] || module_id != header[1] ||
-   req_size != mbox_size) {
-   /*
-* If the message in mailbox is not what we want, we need to
-* clear the mailbox by reading its contents.
-*/
-   for (i = 0; i < mbox_size; i++)
-   if (cdns_mhdp_mailbox_read(mhdp) < 0)
-   break;
-
-   return -EINVAL;
-   }
-
-   return 0;
-}
-
-static int cdns_mhdp_mailbox_recv_data(struct cdns_mhdp_device *mhdp,
-  u8 *buff, u16 buff_size)
-{
-   u32 i;
-   int ret;
-
-   for (i = 0; i < buff_size; i++) {
-   ret = cdns_mhdp_mailbox_read(mhdp);
-   if (ret < 0)
-   return ret;
-
-   buff[i] = ret;
-   }
-
-   return 0;
-}
-
-static int cdns_mhdp_mailbox_send(struct cdns_mhdp_device *mhdp, u8 module_id,
- u8 opcode, u16 size, u8 *message)
-{
-   u8 header[4];
-   int ret, i;
-
-   header[0] = opcode;
-   header[1] = module_id;
-   put_unaligned_be16(size, header + 2);
-
-   for (i = 0; i < sizeof(header); i++) {
-   ret = cdns_mhdp_mailbox_write(mhdp, header[i]);
-   if (ret)
-   return ret;
-   }
-
-   for (i = 0; i < size; i++) {
-   ret = cdns_mhdp_mailbox_write(mhdp, message[i]);
-   if (ret)
-   return ret;
-   }
-
-   return 0;
-}
-
-static
-int cdns_mhdp_reg_read(struct cdns_mhdp_device *mhdp, u32 addr, u32 *value)
-{
-   u8 msg[4], resp[8];
-   int ret;
-
-   put_unaligned_be32(addr, msg);
-
-   mutex_lock(>mbox_mutex);
-
-   ret = cdns_mhdp_mailbox_send(mhdp, MB_MODULE_ID_GENERAL,
-GENERAL_REGISTER_READ,
-sizeof(msg), msg);
-   if (ret)
-   goto out;
-
-   ret = cdns_mhdp_mailbox_recv_header(mhdp, MB_MODULE_ID_GENERAL,
-   GENERAL_REGISTER_READ,
-   sizeof(resp));
-   if (ret)
-   goto out;
-
-   ret = cdns_mhdp_mailbox_recv_data(mhdp, resp, sizeof(resp));
-   if (ret)
-   goto out;
-
-   /* Returned address value should be the same as requested */
-   if (memcmp(msg, resp,

[PATCH v9 0/7] Initial support Cadence MHDP8501(HDMI/DP) for i.MX8MQ

The patch set initial support Cadence MHDP8501(HDMI/DP) DRM bridge
drivers and Cadence HDP-TX PHY(HDMI/DP) drivers for Freescale i.MX8MQ.

The patch set compose of DRM bridge drivers and PHY drivers.

Both of them need the followed two patches to pass build.
drm: bridge: Cadence: convert mailbox functions to macro functions
phy: Add HDMI configuration options

DRM bridges driver patches:
dt-bindings: display: bridge: Add Cadence MHDP850
drm: bridge: Cadence: Add MHDP8501 DP/HDMI driver

PHY driver patches:
dt-bindings: phy: Add Freescale iMX8MQ DP and HDMI PHY
phy: freescale: Add DisplayPort PHY driver for i.MX8MQ
phy: freescale: Add HDMI PHY driver for i.MX8MQ

v8->v9:
- Remove compatible string "cdns,mhdp8501" that had removed
from dt-bindings file in v8.
- Add Dmitry's R-b tag to patch #2
- Add Krzysztof's R-b tag to patch #3

v7->v8:
MHDP8501 HDMI/DP:
- Correct DT node name to "display-bridge".
- Remove "cdns,mhdp8501" from mhdp8501 dt-binding doc.

HDMI/DP PHY:
- Introduced functions `wait_for_ack` and `wait_for_ack_clear` to handle
waiting with acknowledgment bits set and cleared respectively.
- Use FIELD_PRE() to set bitfields for both HDMI and DP PHY.

v6->v7:
MHDP8501 HDMI/DP:
- Combine HDMI and DP driver into one mhdp8501 driver.
Use the connector type to load the corresponding functions.
- Remove connector init functions.
- Add in phy_hdmi.h to reuse ‘enum hdmi_colorspace’.

HDMI/DP PHY:
- Lowercase hex values
- Fix parameters indent issue on some functions
- Replace ‘udelay’ with ‘usleep_range’

v5->v6:
HDMI/DP bridge driver
- 8501 is the part number of Cadence MHDP on i.MX8MQ.
Use MHDP8501 to name hdmi/dp drivers and files.
- Add compatible "fsl,imx8mq-mhdp8501-dp" for i.MX8MQ DP driver
- Add compatible "fsl,imx8mq-mhdp8501-hdmi" for i.MX8MQ HDMI driver
- Combine HDMI and DP dt-bindings into one file cdns,mhdp8501.yaml
- Fix HDMI scrambling is not enable issue when driver working in 4Kp60
mode.
- Add HDMI/DP PHY API mailbox protect.

HDMI/DP PHY driver:
- Rename DP and HDMI PHY files and move to folder phy/freescale/
- Remove properties num_lanes and link_rate from DP PHY driver.
- Combine HDMI and DP dt-bindings into one file fsl,imx8mq-dp-hdmi-phy.yaml
- Update compatible string to "fsl,imx8mq-dp-phy".
- Update compatible string to "fsl,imx8mq-hdmi-phy".

v4->v5:
- Drop "clk" suffix in clock name.
- Add output port property in the example of hdmi/dp.

v3->v4:
dt-bindings:
- Correct dt-bindings coding style and address review comments.
- Add apb_clk description.
- Add output port for HDMI/DP connector
PHY:
- Alphabetically sorted in Kconfig and Makefile for DP and HDMI PHY
- Remove unused registers define from HDMI and DP PHY drivers.
- More description in phy_hdmi.h.
- Add apb_clk to HDMI and DP phy driver.
HDMI/DP:
- Use get_unaligned_le32() to replace hardcode type conversion
in HDMI AVI infoframe data fill function.
- Add mailbox mutex lock in HDMI/DP driver for phy functions
to reslove race conditions between HDMI/DP and PHY drivers.
- Add apb_clk to both HDMI and DP driver.
- Rename some function names and add prefix with "cdns_hdmi/cdns_dp".
- Remove bpc 12 and 16 optional that not supported.

v2->v3:
Address comments for dt-bindings files.
- Correct dts-bindings file names
Rename phy-cadence-hdptx-dp.yaml to cdns,mhdp-imx8mq-dp.yaml
Rename phy-cadence-hdptx-hdmi.yaml to cdns,mhdp-imx8mq-hdmi.yaml
- Drop redundant words and descriptions.
- Correct hdmi/dp node name.

v2 is a completely different version compared to v1.
Previous v1 can be available here [1].

v1->v2:
- Reuse Cadence mailbox access functions from mhdp8546 instead of
rockchip DP.
- Mailbox access functions be convert to marco functions
that will be referenced by HDP-TX PHY(HDMI/DP) driver too.
- Plain bridge instead of component driver.
- Standalone Cadence HDP-TX PHY(HDMI/DP) driver.
- Audio driver are removed from the patch set, it will be add in another
patch set later.

[1]
https://patchwork.kernel.org/project/linux-rockchip/cover/cover.1590982881.git.sandor...@nxp.com/

Sandor Yu (7):
drm: bridge: Cadence: convert mailbox functions to macro functions
phy: Add HDMI configuration options
dt-bindings: display: bridge: Add Cadence MHDP850
drm: bridge: Cadence: Add MHDP8501 DP/HDMI driver
dt-bindings: phy: Add Freescale iMX8MQ DP and HDMI PHY
phy: freescale: Add DisplayPort PHY driver for i.MX8MQ
phy: freescale: Add HDMI PHY driver for i.MX8MQ

.../display/bridge/cdns,mhdp8501.yaml | 104 ++
.../bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml | 53 +
drivers/gpu/drm/bridge/cadence/Kconfig| 15 +
drivers/gpu/drm/bridge/cadence/Makefile | 2 +
.../drm/bridge/cadence/cdns-mhdp8501-core.c | 312 ++
.../drm/bridge/cadence/cdns-mhdp8501-core.h | 410
.../gpu/drm/bridge/cadence/cdns-mhdp8501-dp.c | 780 ++
.../drm/bridge/cadence/cdns-mhdp8501-hdmi.c | 674
.../drm/bridge/cadence/cdns-mhdp8546-core.c | 195

[PATCH v1 1/1] drm/i915/pxp: Add drm_dbgs for critical PXP events.

Debugging PXP issues can't even begin without understanding precedding
sequence of events. Add drm_dbg into the most important PXP events.

Signed-off-by: Alan Previn 
---
 drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c |  2 ++
 drivers/gpu/drm/i915/pxp/intel_pxp.c | 10 --
 drivers/gpu/drm/i915/pxp/intel_pxp_irq.c |  4 ++--
 drivers/gpu/drm/i915/pxp/intel_pxp_session.c |  6 +-
 drivers/gpu/drm/i915/pxp/intel_pxp_types.h   |  1 +
 5 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c 
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c
index 5f138de3c14f..61216c4abaec 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_proxy.c
@@ -321,6 +321,7 @@ static int i915_gsc_proxy_component_bind(struct device 
*i915_kdev,
mutex_lock(>proxy.mutex);
gsc->proxy.component = data;
gsc->proxy.component->mei_dev = mei_kdev;
+   gt_dbg(gt, "GSC proxy mei component bound\n");
mutex_unlock(>proxy.mutex);
 
return 0;
@@ -342,6 +343,7 @@ static void i915_gsc_proxy_component_unbind(struct device 
*i915_kdev,
with_intel_runtime_pm(>runtime_pm, wakeref)
intel_uncore_rmw(gt->uncore, HECI_H_CSR(MTL_GSC_HECI2_BASE),
 HECI_H_CSR_IE | HECI_H_CSR_RST, 0);
+   gt_dbg(gt, "GSC proxy mei component unbound\n");
 }
 
 static const struct component_ops i915_gsc_proxy_component_ops = {
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp.c
index dc327cf40b5a..d285f10bbacc 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp.c
@@ -303,6 +303,8 @@ static int __pxp_global_teardown_final(struct intel_pxp 
*pxp)
 
if (!pxp->arb_is_valid)
return 0;
+
+   drm_dbg(>ctrl_gt->i915->drm, "PXP: %s invoked", __func__);
/*
 * To ensure synchronous and coherent session teardown completion
 * in response to suspend or shutdown triggers, don't use a worker.
@@ -324,6 +326,8 @@ static int __pxp_global_teardown_restart(struct intel_pxp 
*pxp)
 
if (pxp->arb_is_valid)
return 0;
+
+   drm_dbg(>ctrl_gt->i915->drm, "PXP: %s invoked", __func__);
/*
 * The arb-session is currently inactive and we are doing a reset and 
restart
 * due to a runtime event. Use the worker that was designed for this.
@@ -414,10 +418,12 @@ int intel_pxp_start(struct intel_pxp *pxp)
int ret = 0;
 
ret = intel_pxp_get_readiness_status(pxp, PXP_READINESS_TIMEOUT);
-   if (ret < 0)
+   if (ret < 0) {
+   drm_dbg(>ctrl_gt->i915->drm, "PXP: explicit %s failed on 
readiness with %d", __func__, ret);
return ret;
-   else if (ret > 1)
+   } else if (ret > 1) {
return -EIO; /* per UAPI spec, user may retry later */
+   }
 
mutex_lock(>arb_mutex);
 
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
index 91e9622c07d0..0637b1d36356 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_irq.c
@@ -40,11 +40,11 @@ void intel_pxp_irq_handler(struct intel_pxp *pxp, u16 iir)
   GEN12_DISPLAY_APP_TERMINATED_PER_FW_REQ_INTERRUPT)) {
/* immediately mark PXP as inactive on termination */
intel_pxp_mark_termination_in_progress(pxp);
-   pxp->session_events |= PXP_TERMINATION_REQUEST | 
PXP_INVAL_REQUIRED;
+   pxp->session_events |= PXP_TERMINATION_REQUEST | 
PXP_INVAL_REQUIRED | PXP_EVENT_TYPE_IRQ;
}
 
if (iir & GEN12_DISPLAY_STATE_RESET_COMPLETE_INTERRUPT)
-   pxp->session_events |= PXP_TERMINATION_COMPLETE;
+   pxp->session_events |= PXP_TERMINATION_COMPLETE | 
PXP_EVENT_TYPE_IRQ;
 
if (pxp->session_events)
queue_work(system_unbound_wq, >session_work);
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c 
b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
index 0a3e66b0265e..2041dd5221e7 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_session.c
@@ -137,8 +137,10 @@ void intel_pxp_terminate(struct intel_pxp *pxp, bool 
post_invalidation_needs_res
 static void pxp_terminate_complete(struct intel_pxp *pxp)
 {
/* Re-create the arb session after teardown handle complete */
-   if (fetch_and_zero(>hw_state_invalidated))
+   if (fetch_and_zero(>hw_state_invalidated)) {
+   drm_dbg(>ctrl_gt->i915->drm, "PXP: %s to create 
arb_session after invalidation", __func__);
pxp_create_arb_session(pxp);
+   }
 
complete_all(>termination);
 }
@@ -157,6 +159,8 @@ static void pxp_session_work(struct work_struct *work)
if (!events)
return;
 
+   drm_dbg(>i915->drm, "PXP: %s invoked with event-flags

[PATCH v4 1/3] drm/i915/pxp/mtl: Update pxp-firmware response timeout

Update the max GSC-fw response time to match updated internal
fw specs. Because this response time is an SLA on the firmware,
not inclusive of i915->GuC->HW handoff latency, when submitting
requests to the GSC fw via intel_gsc_uc_heci_cmd_submit helpers,
start the count after the request hits the GSC command streamer.
Also, move GSC_REPLY_LATENCY_MS definition from pxp header to
intel_gsc_uc_heci_cmd_submit.h since its for any GSC HECI packet.

Signed-off-by: Alan Previn 
---
 .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 20 +--
 .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h |  6 ++
 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h| 11 ++
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c 
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
index 89ed5ee9cded..fe6a2f78cea0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c
@@ -81,8 +81,17 @@ int intel_gsc_uc_heci_cmd_submit_packet(struct intel_gsc_uc 
*gsc, u64 addr_in,
 
i915_request_add(rq);
 
-   if (!err && i915_request_wait(rq, 0, msecs_to_jiffies(500)) < 0)
-   err = -ETIME;
+   if (!err) {
+   /*
+* Start timeout for i915_request_wait only after considering 
one possible
+* pending GSC-HECI submission cycle on the other 
(non-privileged) path.
+*/
+   if (wait_for(i915_request_started(rq), 
GSC_HECI_REPLY_LATENCY_MS))
+   drm_dbg(_uc_to_gt(gsc)->i915->drm,
+   "Delay in gsc-heci-priv submission to 
gsccs-hw");
+   if (i915_request_wait(rq, 0, msecs_to_jiffies(500)) < 0)
+   err = -ETIME;
+   }
 
i915_request_put(rq);
 
@@ -186,6 +195,13 @@ intel_gsc_uc_heci_cmd_submit_nonpriv(struct intel_gsc_uc 
*gsc,
i915_request_add(rq);
 
if (!err) {
+   /*
+* Start timeout for i915_request_wait only after considering 
one possible
+* pending GSC-HECI submission cycle on the other (privileged) 
path.
+*/
+   if (wait_for(i915_request_started(rq), 
GSC_HECI_REPLY_LATENCY_MS))
+   drm_dbg(_uc_to_gt(gsc)->i915->drm,
+   "Delay in gsc-heci-non-priv submission to 
gsccs-hw");
if (i915_request_wait(rq, I915_WAIT_INTERRUPTIBLE,
  msecs_to_jiffies(timeout_ms)) < 0)
err = -ETIME;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h 
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
index 09d3fbdad05a..5ae5c5d9608b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h
@@ -12,6 +12,12 @@ struct i915_vma;
 struct intel_context;
 struct intel_gsc_uc;
 
+#define GSC_HECI_REPLY_LATENCY_MS 350
+/*
+ * Max FW response time is 350ms, but this should be counted from the time the
+ * command has hit the GSC-CS hardware, not the preceding handoff to GuC CTB.
+ */
+
 struct intel_gsc_mtl_header {
u32 validity_marker;
 #define GSC_HECI_VALIDITY_MARKER 0xA578875A
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h 
b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
index 298ad38e6c7d..a4f17b3ea286 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h
@@ -8,16 +8,19 @@
 
 #include 
 
+#include "gt/uc/intel_gsc_uc_heci_cmd_submit.h"
+
 struct intel_pxp;
 
-#define GSC_REPLY_LATENCY_MS 210
+#define GSC_REPLY_LATENCY_MS GSC_HECI_REPLY_LATENCY_MS
 /*
- * Max FW response time is 200ms, to which we add 10ms to account for overhead
- * such as request preparation, GuC submission to hw and pipeline completion 
times.
+ * Max FW response time is 350ms, but this should be counted from the time the
+ * command has hit the GSC-CS hardware, not the preceding handoff to GuC CTB.
  */
 #define GSC_PENDING_RETRY_MAXCOUNT 40
 #define GSC_PENDING_RETRY_PAUSE_MS 50
-#define GSCFW_MAX_ROUND_TRIP_LATENCY_MS (GSC_PENDING_RETRY_MAXCOUNT * 
GSC_PENDING_RETRY_PAUSE_MS)
+#define GSCFW_MAX_ROUND_TRIP_LATENCY_MS (GSC_REPLY_LATENCY_MS + \
+(GSC_PENDING_RETRY_MAXCOUNT * 
GSC_PENDING_RETRY_PAUSE_MS))
 
 #ifdef CONFIG_DRM_I915_PXP
 void intel_pxp_gsccs_fini(struct intel_pxp *pxp);
-- 
2.39.0

[PATCH v4 3/3] drm/i915/lrc: User PXP contexts requires runalone bit in lrc

On Meteorlake onwards, HW specs require that all user contexts that
run on render or compute engines and require PXP must enforce
run-alone bit in lrc. Add this enforcement for protected contexts.

Signed-off-by: Alan Previn 
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 967fe4d77a87..3df32177e49e 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -845,6 +845,27 @@ lrc_setup_indirect_ctx(u32 *regs,
lrc_ring_indirect_offset_default(engine) << 6;
 }
 
+static bool ctx_needs_runalone(const struct intel_context *ce)
+{
+   struct i915_gem_context *gem_ctx;
+   bool ctx_is_protected = false;
+
+   /*
+* On MTL and newer platforms, protected contexts require setting
+* the LRC run-alone bit or else the encryption will not happen.
+*/
+   if (GRAPHICS_VER_FULL(ce->engine->i915) >= IP_VER(12, 70) &&
+   (ce->engine->class == COMPUTE_CLASS || ce->engine->class == 
RENDER_CLASS)) {
+   rcu_read_lock();
+   gem_ctx = rcu_dereference(ce->gem_context);
+   if (gem_ctx)
+   ctx_is_protected = gem_ctx->uses_protected_content;
+   rcu_read_unlock();
+   }
+
+   return ctx_is_protected;
+}
+
 static void init_common_regs(u32 * const regs,
 const struct intel_context *ce,
 const struct intel_engine_cs *engine,
@@ -860,6 +881,8 @@ static void init_common_regs(u32 * const regs,
if (GRAPHICS_VER(engine->i915) < 11)
ctl |= _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT |
   CTX_CTRL_RS_CTX_ENABLE);
+   if (ctx_needs_runalone(ce))
+   ctl |= _MASKED_BIT_ENABLE(BIT(7));
regs[CTX_CONTEXT_CONTROL] = ctl;
 
regs[CTX_TIMESTAMP] = ce->stats.runtime.last;
-- 
2.39.0

[PATCH v4 0/3] drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec

For MTL, update the GSC-HECI packet size and the max firmware
response timeout to match internal fw specs. Enforce setting
run-alone bit in LRC for protected contexts.

Changes from prio revs:
   v3: - Patch #1. Only start counting the request completion
 timeout from after the request has started (Daniele).
   v2: - Patch #3: fix sparse warning reported by kernel test robot.
   v1: - N/A (Re-test)

Signed-off-by: Alan Previn 

Alan Previn (3):
  drm/i915/pxp/mtl: Update pxp-firmware response timeout
  drm/i915/pxp/mtl: Update pxp-firmware packet size
  drm/i915/lrc: User PXP contexts requires runalone bit in lrc

 drivers/gpu/drm/i915/gt/intel_lrc.c   | 23 +++
 .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 20 ++--
 .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h |  6 +
 .../drm/i915/pxp/intel_pxp_cmd_interface_43.h |  4 ++--
 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h| 11 +
 5 files changed, 56 insertions(+), 8 deletions(-)


base-commit: 5008076127a9599704e98fb4de3761743d943dd0
-- 
2.39.0

[PATCH v4 2/3] drm/i915/pxp/mtl: Update pxp-firmware packet size

Update the GSC-fw input/output HECI packet size to match
updated internal fw specs.

Signed-off-by: Alan Previn 
---
 drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h 
b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
index 0165d38fbead..b2196b008f26 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_cmd_interface_43.h
@@ -14,8 +14,8 @@
 #define PXP43_CMDID_NEW_HUC_AUTH 0x003F /* MTL+ */
 #define PXP43_CMDID_INIT_SESSION 0x0036
 
-/* PXP-Packet sizes for MTL's GSCCS-HECI instruction */
-#define PXP43_MAX_HECI_INOUT_SIZE (SZ_32K)
+/* PXP-Packet sizes for MTL's GSCCS-HECI instruction is 65K*/
+#define PXP43_MAX_HECI_INOUT_SIZE (SZ_64K + SZ_1K)
 
 /* PXP-Packet size for MTL's NEW_HUC_AUTH instruction */
 #define PXP43_HUC_AUTH_INOUT_SIZE (SZ_4K)
-- 
2.39.0

Re: [PATCH drm-misc-next v2 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

2023-09-06 Thread kernel test robot

Hi Danilo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6bd3d8da51ca1ec97c724016466606aec7739b9f]

url:
https://github.com/intel-lab-lkp/linux/commits/Danilo-Krummrich/drm-gpuva_mgr-allow-building-as-module/20230907-054931
base:   6bd3d8da51ca1ec97c724016466606aec7739b9f
patch link:https://lore.kernel.org/r/20230906214723.4393-7-dakr%40redhat.com
patch subject: [PATCH drm-misc-next v2 6/7] drm/gpuvm: generalize 
dma_resv/extobj handling and GEM validation
config: riscv-defconfig 
(https://download.01.org/0day-ci/archive/20230907/202309070844.arxmnmra-...@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20230907/202309070844.arxmnmra-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202309070844.arxmnmra-...@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/drm_gpuva_mgr.c:998: warning: Function parameter or member 
>> 'exec' not described in 'drm_gpuvm_resv_add_fence'


vim +998 drivers/gpu/drm/drm_gpuva_mgr.c

   983  
   984  /**
   985   * drm_gpuvm_resv_add_fence - add fence to private and all extobj
   986   * dma-resv
   987   * @gpuvm: the _gpuvm to add a fence to
   988   * @fence: fence to add
   989   * @private_usage: private dma-resv usage
   990   * @extobj_usage: extobj dma-resv usage
   991   */
   992  void
   993  drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm,
   994   struct drm_exec *exec,
   995   struct dma_fence *fence,
   996   enum dma_resv_usage private_usage,
   997   enum dma_resv_usage extobj_usage)
 > 998  {
   999  struct drm_gem_object *obj;
  1000  unsigned long index;
  1001  
  1002  drm_exec_for_each_locked_object(exec, index, obj) {
  1003  dma_resv_assert_held(obj->resv);
  1004  dma_resv_add_fence(obj->resv, fence,
  1005 drm_gpuvm_is_extobj(gpuvm, 
obj) ?
  1006 private_usage : 
extobj_usage);
  1007  }
  1008  }
  1009  EXPORT_SYMBOL_GPL(drm_gpuvm_resv_add_fence);
  1010  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [PATCH v3 2/5] drm/kmb: add trailing newlines to drm_dbg msgs

2023-09-06 Thread Chrisanthus, Anitha

Acked-by: Anitha Chrisanthus 

> -Original Message-
> From: Jim Cromie 
> Sent: Wednesday, September 6, 2023 12:02 PM
> To: linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org; amd-
> g...@lists.freedesktop.org; intel-gvt-...@lists.freedesktop.org; intel-
> g...@lists.freedesktop.org
> Cc: daniel.vet...@ffwll.ch; dan...@ffwll.ch; Nikula, Jani
> ; ville.syrj...@linux.intel.com;
> seanp...@chromium.org; robdcl...@gmail.com; Jim Cromie
> ; Chrisanthus, Anitha
> ; Edmund Dea ;
> David Airlie 
> Subject: [PATCH v3 2/5] drm/kmb: add trailing newlines to drm_dbg msgs
> 
> By at least strong convention, a print-buffer's trailing newline says
> "message complete, send it".  The exception (no TNL, followed by a call
> to pr_cont) proves the general rule.
> 
> Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
> 1288 drm_dbg.  Clean up the remainders, in maintainer sized chunks.
> 
> No functional changes.
> 
> Signed-off-by: Jim Cromie 
> ---
>  drivers/gpu/drm/kmb/kmb_crtc.c  | 10 +-
>  drivers/gpu/drm/kmb/kmb_plane.c |  6 +++---
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c
> b/drivers/gpu/drm/kmb/kmb_crtc.c
> index 647872f65bff..a58baf25322d 100644
> --- a/drivers/gpu/drm/kmb/kmb_crtc.c
> +++ b/drivers/gpu/drm/kmb/kmb_crtc.c
> @@ -94,7 +94,7 @@ static void kmb_crtc_set_mode(struct drm_crtc *crtc,
>   vm.hback_porch = 0;
>   vm.hsync_len = 28;
> 
> - drm_dbg(dev, "%s : %dactive height= %d vbp=%d vfp=%d vsync-w=%d
> h-active=%d h-bp=%d h-fp=%d hsync-l=%d",
> + drm_dbg(dev, "%s : %dactive height= %d vbp=%d vfp=%d vsync-w=%d
> h-active=%d h-bp=%d h-fp=%d hsync-l=%d\n",
>   __func__, __LINE__,
>   m->crtc_vdisplay, vm.vback_porch, vm.vfront_porch,
>   vm.vsync_len, m->crtc_hdisplay, vm.hback_porch,
> @@ -194,24 +194,24 @@ static enum drm_mode_status
>   int vfp = mode->vsync_start - mode->vdisplay;
> 
>   if (mode->vdisplay < KMB_CRTC_MAX_HEIGHT) {
> - drm_dbg(dev, "height = %d less than %d",
> + drm_dbg(dev, "height = %d less than %d\n",
>   mode->vdisplay, KMB_CRTC_MAX_HEIGHT);
>   return MODE_BAD_VVALUE;
>   }
>   if (mode->hdisplay < KMB_CRTC_MAX_WIDTH) {
> - drm_dbg(dev, "width = %d less than %d",
> + drm_dbg(dev, "width = %d less than %d\n",
>   mode->hdisplay, KMB_CRTC_MAX_WIDTH);
>   return MODE_BAD_HVALUE;
>   }
>   refresh = drm_mode_vrefresh(mode);
>   if (refresh < KMB_MIN_VREFRESH || refresh > KMB_MAX_VREFRESH) {
> - drm_dbg(dev, "refresh = %d less than %d or greater than %d",
> + drm_dbg(dev, "refresh = %d less than %d or greater than
> %d\n",
>   refresh, KMB_MIN_VREFRESH, KMB_MAX_VREFRESH);
>   return MODE_BAD;
>   }
> 
>   if (vfp < KMB_CRTC_MIN_VFP) {
> - drm_dbg(dev, "vfp = %d less than %d", vfp,
> KMB_CRTC_MIN_VFP);
> + drm_dbg(dev, "vfp = %d less than %d\n", vfp,
> KMB_CRTC_MIN_VFP);
>   return MODE_BAD;
>   }
> 
> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c
> b/drivers/gpu/drm/kmb/kmb_plane.c
> index 9e0562aa2bcb..308bd1cb50c8 100644
> --- a/drivers/gpu/drm/kmb/kmb_plane.c
> +++ b/drivers/gpu/drm/kmb/kmb_plane.c
> @@ -78,7 +78,7 @@ static unsigned int check_pixel_format(struct drm_plane
> *plane, u32 format)
>* plane configuration is not supported.
>*/
>   if (init_disp_cfg.format && init_disp_cfg.format != format) {
> - drm_dbg(>drm, "Cannot change format after initial
> plane configuration");
> + drm_dbg(>drm, "Cannot change format after initial
> plane configuration\n");
>   return -EINVAL;
>   }
>   for (i = 0; i < plane->format_count; i++) {
> @@ -124,7 +124,7 @@ static int kmb_plane_atomic_check(struct drm_plane
> *plane,
>   if ((init_disp_cfg.width && init_disp_cfg.height) &&
>   (init_disp_cfg.width != fb->width ||
>   init_disp_cfg.height != fb->height)) {
> - drm_dbg(>drm, "Cannot change plane height or width
> after initial configuration");
> + drm_dbg(>drm, "Cannot change plane height or width
> after initial configuration\n");
>   return -EINVAL;
>   }
>   can_position = (plane->type == DRM_PLANE_TYPE_OVERLAY);
> @@ -375,7 +375,7 @@ static void kmb_plane_atomic_update(struct
> drm_plane *plane,
>   spin_lock_irq(>irq_lock);
>   if (kmb->kmb_under_flow || kmb->kmb_flush_done) {
>   spin_unlock_irq(>irq_lock);
> - drm_dbg(>drm, "plane_update:underflow
> returning");
> + drm_dbg(>drm, "plane_update:underflow
> returning\n");
>   return;
>   }
>   spin_unlock_irq(>irq_lock);
> --
> 2.41.0

[PATCH v4 0/3] drm/i915/pxp/mtl: Update gsc-heci cmd submission to align with fw/hw spec

For MTL, update the GSC-HECI packet size and the max firmware
response timeout to match internal fw specs. Enforce setting
run-alone bit in LRC for protected contexts.

Changes from prio revs:
   v3: - Patch #1. Only start counting the request completion
 timeout from after the request has started (Daniele).
   v2: - Patch #3: fix sparse warning reported by kernel test robot.
   v1: - N/A (Re-test)

Signed-off-by: Alan Previn 

Alan Previn (3):
  drm/i915/pxp/mtl: Update pxp-firmware response timeout
  drm/i915/pxp/mtl: Update pxp-firmware packet size
  drm/i915/lrc: User PXP contexts requires runalone bit in lrc

 drivers/gpu/drm/i915/gt/intel_lrc.c   | 23 +++
 .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.c | 20 ++--
 .../i915/gt/uc/intel_gsc_uc_heci_cmd_submit.h |  6 +
 .../drm/i915/pxp/intel_pxp_cmd_interface_43.h |  4 ++--
 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.h| 11 +
 5 files changed, 56 insertions(+), 8 deletions(-)


base-commit: 5008076127a9599704e98fb4de3761743d943dd0
-- 
2.39.0

Re: [PATCH RFC 2/2] drm: add documentation for drm_buddy_test kUnit test

2023-09-06 Thread Rae Moar

On Fri, Sep 1, 2023 at 3:11 AM Mauro Carvalho Chehab  wrote:
>
> Hi Rae,
>
> Em Thu, 13 Jul 2023 17:31:19 -0400
> Rae Moar  escreveu:
>
> > On Wed, Jul 12, 2023 at 10:29 AM Mauro Carvalho Chehab 
> > wrote:
> >
> > > As an example for the new documentation tool, add a documentation
> > > for drm_buddy_test.
> > >
> > > I opted to place this on a completely different directory, in order
> > > to make easier to test the feature with:
> > >
> > > $ make SPHINXDIRS="tests" htmldocs
> > >
> > > Signed-off-by: Mauro Carvalho Chehab 
> > > ---
> > >
> > > To avoid mailbombing on a large number of people, only mailing lists were
> > > C/C on the cover.
> > > See [PATCH RFC 0/2] at:
> > > https://lore.kernel.org/all/cover.1689171160.git.mche...@kernel.org/
> > >
> > >  Documentation/index.rst|  2 +-
> > >  Documentation/tests/index.rst  |  6 ++
> > >  Documentation/tests/kunit.rst  |  5 +
> > >  drivers/gpu/drm/tests/drm_buddy_test.c | 12 
> > >  4 files changed, 24 insertions(+), 1 deletion(-)
> > >  create mode 100644 Documentation/tests/index.rst
> > >  create mode 100644 Documentation/tests/kunit.rst
> > >
> > > diff --git a/Documentation/index.rst b/Documentation/index.rst
> > > index 9dfdc826618c..80a6ce14a61a 100644
> > > --- a/Documentation/index.rst
> > > +++ b/Documentation/index.rst
> > > @@ -60,7 +60,7 @@ Various other manuals with useful information for all
> > > kernel developers.
> > > fault-injection/index
> > > livepatch/index
> > > rust/index
> > > -
> > > +   test/index
> > >
> > >  User-oriented documentation
> > >  ===
> > > diff --git a/Documentation/tests/index.rst b/Documentation/tests/index.rst
> > > new file mode 100644
> > > index ..bfc39eb5c0aa
> > > --- /dev/null
> > > +++ b/Documentation/tests/index.rst
> > > @@ -0,0 +1,6 @@
> > > +
> > > +Kunit documentation test
> > > +
> > > +
> > > +.. toctree::
> > > +   kunit
> > > diff --git a/Documentation/tests/kunit.rst b/Documentation/tests/kunit.rst
> > > new file mode 100644
> > > index ..6ffc151988a0
> > > --- /dev/null
> > > +++ b/Documentation/tests/kunit.rst
> > > @@ -0,0 +1,5 @@
> > > +Kunit tests
> > > +---
> > > +
> > > +.. include-test:: drivers/gpu/drm/tests/drm_buddy_test.c
> > > +
> > > diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
> > > b/drivers/gpu/drm/tests/drm_buddy_test.c
> > > index 09ee6f6af896..dd6c5afd6cd6 100644
> > > --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> > > +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> > > @@ -737,6 +737,18 @@ static int drm_buddy_suite_init(struct kunit_suite
> > > *suite)
> > > return 0;
> > >  }
> > >
> > > +/**
> > > + * KTEST_SUITE: set of tests for drm buddy alloc
> > > + * Scope: drm subsystem
> > > + * Mega feature: drm
> > > + * Feature: buddy_alloc
> > > + *
> > > + * KTEST_TEST: drm_test_buddy_alloc_%s
> > > + * Description: Run DRM buddy allocation %arg[1] test
> > > + *
> > > + * arg[1].values: limit, range, optimistic, smoke, pathological
> > > + */
> >
> >
> > Hi!
> >
> > This is such a cool patch series. I just have a few comments related to the
> > output.
>
> Thank you for your comments! Sorry for not answering earlier. I took some
> vacations and this series ended sleeping over other tasks on my
> todo list.
>
> Also, before sending another version, I wanted to have the test_list.py
> changes to make it generic enough to be merged on IGT, to avoid having
> a fork of it. Those got merged today.

Hi Mauro!

No worries at all!

>
> > In the html output the tests are listed as:
> > ktest@drm_buddy_test@…
> >
> > I wonder if instead of using the file name of “drm_buddy_test” this could
> > possibly be the suite name, “drm_buddy”, as this is what users will call
> > when using kunit.py to run the tests. Although "drm_buddy_test" is also the
> > module name so I don't mind it too much. But in the future the file name
> > and module name are not guaranteed to be the same for other tests.
> >
> > Most preferably, there would be a reference to the kunit suite name, file
> > name, and the module name.
>
> I guess it shouldn't be hard to do such change in a way that it won't
> affect its usage on IGT. We need to define what would be the best
> pattern. As this can be used for both kunit and selftests, I would
> place kunit at the beginning.
>
> Currently, we're using:
>
> kunit@@
>
> Some possible patterns would be:
>
> kunit@@@
> kunit
> kunit@@@
>
> Would do you think it would work best?

How possible is it to separate out the file and suite name as headers?
I think that this could reduce some of the repetition.

If we are already separating documentation pages by subsystem, a
potential format could be:

File: 

 suite: 
List of Tests:


...

What do you think?

>
> > This may be difficult to implement as these can all differ. I am currently
> > working on

Re: [PATCH 4/6] drm/msm: add a kernel param to select between MDP5 and DPU drivers

Quoting Dmitry Baryshkov (2023-09-05 10:43:51)
> For some of the platforms (e.g. SDM660, SDM630, MSM8996, etc.) it is
> possible to support this platform via the DPU driver (e.g. to provide
> support for DP, multirect, etc). Add a modparam to be able to switch
> between these two drivers.
>
> All platforms supported by both drivers are by default handled by the
> MDP5 driver. To let them be handled by the DPU driver pass the
> `msm.prefer_mdp5=false` kernel param.
>
> Signed-off-by: Dmitry Baryshkov 
> ---

Reviewed-by: Stephen Boyd

Re: [PATCH 3/6] drm/msm/dpu: support binding to the mdp5 devices

2023-09-06 Thread Dmitry Baryshkov

On Thu, 7 Sept 2023 at 01:17, Stephen Boyd  wrote:
>
> Quoting Dmitry Baryshkov (2023-09-05 10:43:50)
> > diff --git a/drivers/gpu/drm/msm/msm_io_utils.c 
> > b/drivers/gpu/drm/msm/msm_io_utils.c
> > index 59d2788c4510..9d0d76f3a319 100644
> > --- a/drivers/gpu/drm/msm/msm_io_utils.c
> > +++ b/drivers/gpu/drm/msm/msm_io_utils.c
> > @@ -50,6 +50,24 @@ struct clk *msm_clk_get(struct platform_device *pdev, 
> > const char *name)
> > return clk;
> >  }
> >
> > +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev,
> > +  struct platform_device *pdev,
> > +  const char *name)
> > +{
> > +   struct resource *res;
> > +   void __iomem *ptr;
> > +
> > +   res = platform_get_resource_byname(mdss_pdev, IORESOURCE_MEM, name);
> > +   if (!res)
> > +   return ERR_PTR(-EINVAL);
> > +
> > +   ptr = devm_ioremap_resource(>dev, res);
> > +   if (!ptr)
>
> devm_ioremap_resource() returns an error pointer. Too bad we can't use
> devm_platform_ioremap_resource_byname() here.

Unfortunately :-(

>
> > +   return ERR_PTR(-ENOMEM);
> > +
> > +   return ptr;
> > +}



-- 
With best wishes
Dmitry

Re: [PATCH 2/6] drm/msm/mdss: generate MDSS data for MDP5 platforms

2023-09-06 Thread Dmitry Baryshkov

On Thu, 7 Sept 2023 at 01:10, Stephen Boyd  wrote:
>
> Quoting Dmitry Baryshkov (2023-09-05 10:43:49)
> > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> > index 348c66b14683..fb6ee93b5abc 100644
> > --- a/drivers/gpu/drm/msm/msm_mdss.c
> > +++ b/drivers/gpu/drm/msm/msm_mdss.c
> > @@ -222,6 +222,36 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss 
> > *msm_mdss)
> > }
> >  }
> >
> > +static struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct 
> > msm_mdss *mdss)
>
> const mdss?

Ack

>
> > +{
> > +   struct msm_mdss_data *data;
> > +   u32 hw_rev;
> > +
> > +   data = devm_kzalloc(mdss->dev, sizeof(*data), GFP_KERNEL);
> > +   if (!data)
> > +   return NULL;
> > +
> > +   hw_rev = readl_relaxed(mdss->mmio + HW_REV) >> 16;
>
> Or like this
>
> hw_rev = upper_16_bits(readl_relaxed(...))

Ugh. That looks ugly, I'd say. >> 16 is more common.

>
> > +
> > +   if (hw_rev == 0x1007 || /* msm8996 */
> > +   hw_rev == 0x100e || /* msm8937 */
> > +   hw_rev == 0x1010 || /* msm8953 */
> > +   hw_rev == 0x3000 || /* msm8998 */
> > +   hw_rev == 0x3002 || /* sdm660 */
> > +   hw_rev == 0x3003) { /* sdm630 */
>
> Can we have #defines for hw_revs and drop the comments?
>
> > +   data->ubwc_dec_version = UBWC_1_0;
> > +   data->ubwc_enc_version = UBWC_1_0;
> > +   }
> > +
> > +   if (hw_rev == 0x1007 || /* msm8996 */
> > +   hw_rev == 0x3000) /* msm8998 */
>
> Then we don't have to worry about these two having typos.

Ack

>
> > +   data->highest_bank_bit = 2;
> > +   else
> > +   data->highest_bank_bit = 1;
> >



-- 
With best wishes
Dmitry

Re: [PATCH 3/6] drm/msm/dpu: support binding to the mdp5 devices

Quoting Dmitry Baryshkov (2023-09-05 10:43:50)
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c 
> b/drivers/gpu/drm/msm/msm_io_utils.c
> index 59d2788c4510..9d0d76f3a319 100644
> --- a/drivers/gpu/drm/msm/msm_io_utils.c
> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
> @@ -50,6 +50,24 @@ struct clk *msm_clk_get(struct platform_device *pdev, 
> const char *name)
> return clk;
>  }
>
> +void __iomem *msm_ioremap_mdss(struct platform_device *mdss_pdev,
> +  struct platform_device *pdev,
> +  const char *name)
> +{
> +   struct resource *res;
> +   void __iomem *ptr;
> +
> +   res = platform_get_resource_byname(mdss_pdev, IORESOURCE_MEM, name);
> +   if (!res)
> +   return ERR_PTR(-EINVAL);
> +
> +   ptr = devm_ioremap_resource(>dev, res);
> +   if (!ptr)

devm_ioremap_resource() returns an error pointer. Too bad we can't use
devm_platform_ioremap_resource_byname() here.

> +   return ERR_PTR(-ENOMEM);
> +
> +   return ptr;
> +}

Re: [PATCH v3 2/8] drm/msm/dpu: enable PINGPONG TE operations only when supported by HW

2023-09-06 Thread Dmitry Baryshkov

On Thu, 7 Sept 2023 at 00:54, Stephen Boyd  wrote:
>
> Quoting Dmitry Baryshkov (2023-09-03 19:04:48)
> > The DPU_PINGPONG_TE bit is set for all PINGPONG blocks on DPU < 5.0.
> > Rather than checking for the flag, check for the presense of the
>
> s/presense/presence/
>
> > corresponding interrupt line.
>
> The patch checks for the major version though?

ugh, forgot to update the commit message after rebasing on top of
Abhinav's core_major_rev series. I'll fix this in v4/



-- 
With best wishes
Dmitry

Re: [PATCH 2/6] drm/msm/mdss: generate MDSS data for MDP5 platforms

Quoting Dmitry Baryshkov (2023-09-05 10:43:49)
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 348c66b14683..fb6ee93b5abc 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -222,6 +222,36 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss 
> *msm_mdss)
> }
>  }
>
> +static struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct 
> msm_mdss *mdss)

const mdss?

> +{
> +   struct msm_mdss_data *data;
> +   u32 hw_rev;
> +
> +   data = devm_kzalloc(mdss->dev, sizeof(*data), GFP_KERNEL);
> +   if (!data)
> +   return NULL;
> +
> +   hw_rev = readl_relaxed(mdss->mmio + HW_REV) >> 16;

Or like this

hw_rev = upper_16_bits(readl_relaxed(...))

> +
> +   if (hw_rev == 0x1007 || /* msm8996 */
> +   hw_rev == 0x100e || /* msm8937 */
> +   hw_rev == 0x1010 || /* msm8953 */
> +   hw_rev == 0x3000 || /* msm8998 */
> +   hw_rev == 0x3002 || /* sdm660 */
> +   hw_rev == 0x3003) { /* sdm630 */

Can we have #defines for hw_revs and drop the comments?

> +   data->ubwc_dec_version = UBWC_1_0;
> +   data->ubwc_enc_version = UBWC_1_0;
> +   }
> +
> +   if (hw_rev == 0x1007 || /* msm8996 */
> +   hw_rev == 0x3000) /* msm8998 */

Then we don't have to worry about these two having typos.

> +   data->highest_bank_bit = 2;
> +   else
> +   data->highest_bank_bit = 1;
>

Re: [PATCH v3 5/8] drm/msm/dpu: enable INTF TE operations only when supported by HW

Quoting Dmitry Baryshkov (2023-09-03 19:04:51)
> The DPU_INTF_TE bit is set for all INTF blocks on DPU >= 5.0, however
> only INTF_1 and INTF_2 actually support tearing control (both are
> INTF_DSI). Rather than trying to limit the DPU_INTF_TE feature bit to
> those two INTF instances, check for the major && INTF type.
>
> Reviewed-by: Marijn Suijten 
> Signed-off-by: Dmitry Baryshkov 
> ---

Reviewed-by: Stephen Boyd

Re: [PATCH v3 6/8] drm/msm/dpu: drop DPU_INTF_TE feature flag

Quoting Dmitry Baryshkov (2023-09-03 19:04:52)
> Replace the only user of the DPU_INTF_TE feature flag with the direct
> DPU version comparison.
>
> Reviewed-by: Marijn Suijten 
> Signed-off-by: Dmitry Baryshkov 
> ---

Reviewed-by: Stephen Boyd

Re: [PATCH v3 7/8] drm/msm/dpu: drop useless check from dpu_encoder_phys_cmd_te_rd_ptr_irq()

Quoting Dmitry Baryshkov (2023-09-03 19:04:53)
> The dpu_encoder_phys_cmd_te_rd_ptr_irq() function uses neither hw_intf
> nor hw_pp data, so we can drop the corresponding check.
>
> Reviewed-by: Marijn Suijten 
> Signed-off-by: Dmitry Baryshkov 
> ---

Reviewed-by: Stephen Boyd

Re: [PATCH v3 8/8] drm/msm/dpu: move INTF tearing checks to dpu_encoder_phys_cmd_init

Quoting Dmitry Baryshkov (2023-09-03 19:04:54)
> As the INTF is fixed at the encoder creation time, we can move the
> check whether INTF supports tearchck to dpu_encoder_phys_cmd_init().
> This function can return an error if INTF doesn't have required feature.
> Performing this check in dpu_encoder_phys_cmd_tearcheck_config() is less
> useful, as this function returns void.
>
> Signed-off-by: Dmitry Baryshkov 
> ---

Reviewed-by: Stephen Boyd

Re: [PATCH v3 4/8] drm/msm/dpu: inline _setup_intf_ops()

Quoting Dmitry Baryshkov (2023-09-03 19:04:50)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> index 8ec6505d9e78..dd67686f5403 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c
> @@ -524,31 +524,6 @@ static void dpu_hw_intf_program_intf_cmd_cfg(struct 
> dpu_hw_intf *ctx,
[...]
> -   ops->disable_autorefresh = dpu_hw_intf_disable_autorefresh;
> -   }
> -
> -   if (mdss_rev->core_major_ver >= 7)
> -   ops->program_intf_cmd_cfg = dpu_hw_intf_program_intf_cmd_cfg;
> -}
> -
>  struct dpu_hw_intf *dpu_hw_intf_init(const struct dpu_intf_cfg *cfg,
> void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
>  {
> @@ -571,7 +546,28 @@ struct dpu_hw_intf *dpu_hw_intf_init(const struct 
> dpu_intf_cfg *cfg,
>  */
> c->idx = cfg->id;
> c->cap = cfg;
> -   _setup_intf_ops(>ops, c->cap->features, mdss_rev);
> +
> +   c->ops.setup_timing_gen = dpu_hw_intf_setup_timing_engine;

Maybe grow a local variable 'ops' that can be assigned to so the code
doesn't change at all, only the location does.

> +   c->ops.setup_prg_fetch  = dpu_hw_intf_setup_prg_fetch;
> +   c->ops.get_status = dpu_hw_intf_get_status;

Re: [PATCH v3 3/8] drm/msm/dpu: drop the DPU_PINGPONG_TE flag

Quoting Dmitry Baryshkov (2023-09-03 19:04:49)
> The DPU_PINGPONG_TE flag became unused, we can drop it now.
>
> Reviewed-by: Marijn Suijten 
> Signed-off-by: Dmitry Baryshkov 
> ---

Reviewed-by: Stephen Boyd

Re: [PATCH v3 2/8] drm/msm/dpu: enable PINGPONG TE operations only when supported by HW

Quoting Dmitry Baryshkov (2023-09-03 19:04:48)
> The DPU_PINGPONG_TE bit is set for all PINGPONG blocks on DPU < 5.0.
> Rather than checking for the flag, check for the presense of the

s/presense/presence/

> corresponding interrupt line.

The patch checks for the major version though?

Re: [PATCH v3 1/8] drm/msm/dpu: inline _setup_pingpong_ops()

Quoting Dmitry Baryshkov (2023-09-03 19:04:47)
> Inline the _setup_pingpong_ops() function, it makes it easier to handle
> different conditions involving PINGPONG configuration.
>
> Reviewed-by: Marijn Suijten 
> Signed-off-by: Dmitry Baryshkov 
> ---

Reviewed-by: Stephen Boyd

[PATCH drm-misc-next v2 7/7] drm/nouveau: GPUVM dma-resv/extobj handling, GEM validation

Make use of the DRM GPUVA managers GPU-VM common dma-resv, external GEM
object tracking, dma-resv locking, evicted GEM object tracking and
validation features.

Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c|  4 +-
 drivers/gpu/drm/nouveau/nouveau_exec.c  | 52 +++--
 drivers/gpu/drm/nouveau/nouveau_exec.h  |  4 -
 drivers/gpu/drm/nouveau/nouveau_gem.c   |  4 +-
 drivers/gpu/drm/nouveau/nouveau_sched.h |  4 +-
 drivers/gpu/drm/nouveau/nouveau_uvmm.c  | 99 -
 6 files changed, 82 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 19cab37ac69c..18c91993dae1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1060,17 +1060,18 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool 
evict,
 {
struct nouveau_drm *drm = nouveau_bdev(bo->bdev);
struct nouveau_bo *nvbo = nouveau_bo(bo);
+   struct drm_gem_object *obj = >base;
struct ttm_resource *old_reg = bo->resource;
struct nouveau_drm_tile *new_tile = NULL;
int ret = 0;
 
-
if (new_reg->mem_type == TTM_PL_TT) {
ret = nouveau_ttm_tt_bind(bo->bdev, bo->ttm, new_reg);
if (ret)
return ret;
}
 
+   drm_gpuvm_bo_evict(obj, evict);
nouveau_bo_move_ntfy(bo, new_reg);
ret = ttm_bo_wait_ctx(bo, ctx);
if (ret)
@@ -1135,6 +1136,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 out_ntfy:
if (ret) {
nouveau_bo_move_ntfy(bo, bo->resource);
+   drm_gpuvm_bo_evict(obj, !evict);
}
return ret;
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c 
b/drivers/gpu/drm/nouveau/nouveau_exec.c
index b4239af29e5a..5f86043046f5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.c
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
@@ -1,7 +1,5 @@
 // SPDX-License-Identifier: MIT
 
-#include 
-
 #include "nouveau_drv.h"
 #include "nouveau_gem.h"
 #include "nouveau_mem.h"
@@ -91,9 +89,6 @@ nouveau_exec_job_submit(struct nouveau_job *job)
struct nouveau_exec_job *exec_job = to_nouveau_exec_job(job);
struct nouveau_cli *cli = job->cli;
struct nouveau_uvmm *uvmm = nouveau_cli_uvmm(cli);
-   struct drm_exec *exec = >exec;
-   struct drm_gem_object *obj;
-   unsigned long index;
int ret;
 
ret = nouveau_fence_new(_job->fence);
@@ -101,52 +96,29 @@ nouveau_exec_job_submit(struct nouveau_job *job)
return ret;
 
nouveau_uvmm_lock(uvmm);
-   drm_exec_init(exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
-   DRM_EXEC_IGNORE_DUPLICATES);
-   drm_exec_until_all_locked(exec) {
-   struct drm_gpuva *va;
-
-   drm_gpuvm_for_each_va(va, >base) {
-   if (unlikely(va == >base.kernel_alloc_node))
-   continue;
-
-   ret = drm_exec_prepare_obj(exec, va->gem.obj, 1);
-   drm_exec_retry_on_contention(exec);
-   if (ret)
-   goto err_uvmm_unlock;
-   }
+   job->vm_exec.vm = >base;
+   ret = drm_gpuvm_exec_lock(>vm_exec, 1, false);
+   if (ret) {
+   nouveau_uvmm_unlock(uvmm);
+   return ret;
}
nouveau_uvmm_unlock(uvmm);
 
-   drm_exec_for_each_locked_object(exec, index, obj) {
-   struct nouveau_bo *nvbo = nouveau_gem_object(obj);
-
-   ret = nouveau_bo_validate(nvbo, true, false);
-   if (ret)
-   goto err_exec_fini;
+   ret = drm_gpuvm_validate(>base);
+   if (ret) {
+   drm_gpuvm_exec_unlock(>vm_exec);
+   return ret;
}
 
return 0;
-
-err_uvmm_unlock:
-   nouveau_uvmm_unlock(uvmm);
-err_exec_fini:
-   drm_exec_fini(exec);
-   return ret;
-
 }
 
 static void
 nouveau_exec_job_armed_submit(struct nouveau_job *job)
 {
-   struct drm_exec *exec = >exec;
-   struct drm_gem_object *obj;
-   unsigned long index;
-
-   drm_exec_for_each_locked_object(exec, index, obj)
-   dma_resv_add_fence(obj->resv, job->done_fence, job->resv_usage);
-
-   drm_exec_fini(exec);
+   drm_gpuvm_exec_resv_add_fence(>vm_exec, job->done_fence,
+ job->resv_usage, job->resv_usage);
+   drm_gpuvm_exec_unlock(>vm_exec);
 }
 
 static struct dma_fence *
diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.h 
b/drivers/gpu/drm/nouveau/nouveau_exec.h
index 778cacd90f65..b815de2428f3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.h
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.h
@@ -3,16 +3,12 @@
 #ifndef __NOUVEAU_EXEC_H__
 #define __NOUVEAU_EXEC_H__
 
-#include 
-
 #include "nouveau_drv.h"
 #include "nouveau_sched.h"
 
 struct nouveau_exec_job_args {

[PATCH drm-misc-next v2 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

So far the DRM GPUVA manager offers common infrastructure to track GPU VA
allocations and mappings, generically connect GPU VA mappings to their
backing buffers and perform more complex mapping operations on the GPU VA
space.

However, there are more design patterns commonly used by drivers, which
can potentially be generalized in order to make the DRM GPUVA manager
represent a basic GPU-VM implementation. In this context, this patch aims
at generalizing the following elements.

1) Provide a common dma-resv for GEM objects not being used outside of
   this GPU-VM.

2) Provide tracking of external GEM objects (GEM objects which are
   shared with other GPU-VMs).

3) Provide functions to efficiently lock all GEM objects dma-resv the
   GPU-VM contains mappings of.

4) Provide tracking of evicted GEM objects the GPU-VM contains mappings
   of, such that validation of evicted GEM objects is accelerated.

5) Provide some convinience functions for common patterns.

Rather than being designed as a "framework", the target is to make all
features appear as a collection of optional helper functions, such that
drivers are free to make use of the DRM GPUVA managers basic
functionality and opt-in for other features without setting any feature
flags, just by making use of the corresponding functions.

Suggested-by: Matthew Brost 
Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/drm_gpuva_mgr.c | 335 +++-
 include/drm/drm_gpuva_mgr.h | 235 +-
 2 files changed, 564 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c
index da7a6e1aabe0..db4ef4fadc4b 100644
--- a/drivers/gpu/drm/drm_gpuva_mgr.c
+++ b/drivers/gpu/drm/drm_gpuva_mgr.c
@@ -678,6 +678,10 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device 
*drm,
gpuvm->rb.tree = RB_ROOT_CACHED;
INIT_LIST_HEAD(>rb.list);
 
+   INIT_LIST_HEAD(>extobj.list);
+   INIT_LIST_HEAD(>evict.list);
+   spin_lock_init(>evict.lock);
+
drm_gpuva_check_overflow(start_offset, range);
gpuvm->mm_start = start_offset;
gpuvm->mm_range = range;
@@ -719,10 +723,291 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
 "GPUVA tree is not empty, potentially leaking memory.\n");
 
+   WARN(!list_empty(>extobj.list), "Extobj list should be 
empty.\n");
+   WARN(!list_empty(>evict.list), "Evict list should be empty.\n");
+
drm_gem_private_object_fini(>d_obj);
 }
 EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
 
+/**
+ * drm_gpuvm_prepare_objects() - prepare all assoiciated BOs
+ * @gpuvm: the _gpuvm
+ * @exec: the _exec locking context
+ * @num_fences: the amount of _fences to reserve
+ *
+ * Calls drm_exec_prepare_obj() for all _gem_objects the given
+ * _gpuvm contains mappings of.
+ *
+ * Using this function directly, it is the drivers responsibility to call
+ * drm_exec_init() and drm_exec_fini() accordingly.
+ *
+ * Returns: 0 on success, negative error code on failure.
+ */
+int
+drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
+ struct drm_exec *exec,
+ unsigned int num_fences)
+{
+   struct drm_gpuvm_bo *vm_bo;
+   int ret;
+
+   list_for_each_entry(vm_bo, >extobj.list, list.entry.extobj) {
+   struct drm_gem_object *obj = vm_bo->obj;
+
+   ret = drm_exec_prepare_obj(exec, obj, num_fences);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_objects);
+
+/**
+ * drm_gpuvm_prepare_range() - prepare all BOs mapped within a given range
+ * @gpuvm: the _gpuvm
+ * @exec: the _exec locking context
+ * @addr: the start address within the VA space
+ * @range: the range to iterate within the VA space
+ * @num_fences: the amount of _fences to reserve
+ *
+ * Calls drm_exec_prepare_obj() for all _gem_objects mapped between @addr
+ * and @addr + @range.
+ *
+ * Returns: 0 on success, negative error code on failure.
+ */
+int
+drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, struct drm_exec *exec,
+   u64 addr, u64 range, unsigned int num_fences)
+{
+   struct drm_gpuva *va;
+   u64 end = addr + range;
+   int ret;
+
+   drm_gpuvm_for_each_va_range(va, gpuvm, addr, end) {
+   struct drm_gem_object *obj = va->gem.obj;
+
+   ret = drm_exec_prepare_obj(exec, obj, num_fences);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_range);
+
+/**
+ * drm_gpuvm_exec_lock() - lock all dma-resv of all assoiciated BOs
+ * @vm_exec: the _gpuvm_exec abstraction
+ * @num_fences: the amount of _fences to reserve
+ * @interruptible: sleep interruptible if waiting
+ *
+ * Acquires all dma-resv locks of all _gem_objects the given
+ * _gpuvm contains mappings of.
+ *
+ * Addionally, when calling this

[PATCH drm-misc-next v2 3/7] drm/nouveau: uvmm: rename 'umgr' to 'base'

Rename struct drm_gpuvm within struct nouveau_uvmm from 'umgr' to base.

Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/nouveau/nouveau_debugfs.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_exec.c|  4 +--
 drivers/gpu/drm/nouveau/nouveau_uvmm.c| 32 +++
 drivers/gpu/drm/nouveau/nouveau_uvmm.h|  6 ++---
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c 
b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 053f703f2f68..e83db051e851 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -231,7 +231,7 @@ nouveau_debugfs_gpuva(struct seq_file *m, void *data)
continue;
 
nouveau_uvmm_lock(uvmm);
-   drm_debugfs_gpuva_info(m, >umgr);
+   drm_debugfs_gpuva_info(m, >base);
seq_puts(m, "\n");
nouveau_debugfs_gpuva_regions(m, uvmm);
nouveau_uvmm_unlock(uvmm);
diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c 
b/drivers/gpu/drm/nouveau/nouveau_exec.c
index c001952cd678..b4239af29e5a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_exec.c
+++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
@@ -106,8 +106,8 @@ nouveau_exec_job_submit(struct nouveau_job *job)
drm_exec_until_all_locked(exec) {
struct drm_gpuva *va;
 
-   drm_gpuvm_for_each_va(va, >umgr) {
-   if (unlikely(va == >umgr.kernel_alloc_node))
+   drm_gpuvm_for_each_va(va, >base) {
+   if (unlikely(va == >base.kernel_alloc_node))
continue;
 
ret = drm_exec_prepare_obj(exec, va->gem.obj, 1);
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 16c7ea3cee13..4c47b2279674 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -329,7 +329,7 @@ nouveau_uvma_region_create(struct nouveau_uvmm *uvmm,
struct nouveau_uvma_region *reg;
int ret;
 
-   if (!drm_gpuva_interval_empty(>umgr, addr, range))
+   if (!drm_gpuva_interval_empty(>base, addr, range))
return -ENOSPC;
 
ret = nouveau_uvma_region_alloc();
@@ -384,7 +384,7 @@ nouveau_uvma_region_empty(struct nouveau_uvma_region *reg)
 {
struct nouveau_uvmm *uvmm = reg->uvmm;
 
-   return drm_gpuva_interval_empty(>umgr,
+   return drm_gpuva_interval_empty(>base,
reg->va.addr,
reg->va.range);
 }
@@ -589,7 +589,7 @@ op_map_prepare(struct nouveau_uvmm *uvmm,
uvma->region = args->region;
uvma->kind = args->kind;
 
-   drm_gpuva_map(>umgr, >va, op);
+   drm_gpuva_map(>base, >va, op);
 
/* Keep a reference until this uvma is destroyed. */
nouveau_uvma_gem_get(uvma);
@@ -1194,7 +1194,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job)
goto unwind_continue;
}
 
-   op->ops = drm_gpuva_sm_unmap_ops_create(>umgr,
+   op->ops = drm_gpuva_sm_unmap_ops_create(>base,
op->va.addr,
op->va.range);
if (IS_ERR(op->ops)) {
@@ -1205,7 +1205,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job)
ret = nouveau_uvmm_sm_unmap_prepare(uvmm, >new,
op->ops);
if (ret) {
-   drm_gpuva_ops_free(>umgr, op->ops);
+   drm_gpuva_ops_free(>base, op->ops);
op->ops = NULL;
op->reg = NULL;
goto unwind_continue;
@@ -1240,7 +1240,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job)
}
}
 
-   op->ops = drm_gpuva_sm_map_ops_create(>umgr,
+   op->ops = drm_gpuva_sm_map_ops_create(>base,
  op->va.addr,
  op->va.range,
  op->gem.obj,
@@ -1256,7 +1256,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job)
  op->va.range,
  op->flags & 0xff);
if (ret) {
-   drm_gpuva_ops_free(>umgr, op->ops);
+   drm_gpuva_ops_free(>base, op->ops);
op->ops = NULL;
goto unwind_continue;

[PATCH drm-misc-next v2 5/7] drm/gpuvm: add an abstraction for a VM / BO combination

This patch adds an abstraction layer between the drm_gpuva mappings of
a particular drm_gem_object and this GEM object itself. The abstraction
represents a combination of a drm_gem_object and drm_gpuvm. The
drm_gem_object holds a list of drm_gpuvm_bo structures (the structure
representing this abstraction), while each drm_gpuvm_bo contains list of
mappings of this GEM object.

This has multiple advantages:

1) We can use the drm_gpuvm_bo structure to attach it to various lists
   of the drm_gpuvm. This is useful for tracking external and evicted
   objects per VM, which is introduced in subsequent patches.

2) Finding mappings of a certain drm_gem_object mapped in a certain
   drm_gpuvm becomes much cheaper.

3) Drivers can derive and extend the structure to easily represent
   driver specific states of a BO for a certain GPUVM.

The idea of this abstraction was taken from amdgpu, hence the credit for
this idea goes to the developers of amdgpu.

Cc: Christian König 
Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/drm_gpuva_mgr.c| 210 +++--
 drivers/gpu/drm/nouveau/nouveau_uvmm.c |  77 ++---
 include/drm/drm_gem.h  |  48 --
 include/drm/drm_gpuva_mgr.h| 153 +-
 4 files changed, 437 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c
index 838277794990..da7a6e1aabe0 100644
--- a/drivers/gpu/drm/drm_gpuva_mgr.c
+++ b/drivers/gpu/drm/drm_gpuva_mgr.c
@@ -723,6 +723,161 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
 }
 EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
 
+/**
+ * drm_gpuvm_bo_create() - create a new instance of struct drm_gpuvm_bo
+ * @gpuvm: The _gpuvm the @obj is mapped in.
+ * @obj: The _gem_object being mapped in the @gpuvm.
+ *
+ * If provided by the driver, this function uses the _gpuvm_ops
+ * vm_bo_alloc() callback to allocate.
+ *
+ * Returns: a pointer to the _gpuvm_bo on success, NULL on failure
+ */
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_create(struct drm_gpuvm *gpuvm,
+   struct drm_gem_object *obj)
+{
+   const struct drm_gpuvm_ops *ops = gpuvm->ops;
+   struct drm_gpuvm_bo *vm_bo;
+
+   if (ops && ops->vm_bo_alloc)
+   vm_bo = ops->vm_bo_alloc();
+   else
+   vm_bo = kzalloc(sizeof(*vm_bo), GFP_KERNEL);
+
+   if (unlikely(!vm_bo))
+   return NULL;
+
+   vm_bo->vm = gpuvm;
+   vm_bo->obj = obj;
+
+   kref_init(_bo->kref);
+   INIT_LIST_HEAD(_bo->list.gpuva);
+   INIT_LIST_HEAD(_bo->list.entry.gem);
+
+   drm_gem_object_get(obj);
+
+   return vm_bo;
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_create);
+
+void
+drm_gpuvm_bo_destroy(struct kref *kref)
+{
+   struct drm_gpuvm_bo *vm_bo = container_of(kref, struct drm_gpuvm_bo,
+ kref);
+   const struct drm_gpuvm_ops *ops = vm_bo->vm->ops;
+
+   drm_gem_object_put(vm_bo->obj);
+
+   if (ops && ops->vm_bo_free)
+   ops->vm_bo_free(vm_bo);
+   else
+   kfree(vm_bo);
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_destroy);
+
+static struct drm_gpuvm_bo *
+__drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
+   struct drm_gem_object *obj)
+{
+   struct drm_gpuvm_bo *vm_bo;
+
+   drm_gem_gpuva_assert_lock_held(obj);
+
+   drm_gem_for_each_gpuva_gem(vm_bo, obj)
+   if (vm_bo->vm == gpuvm)
+   return vm_bo;
+
+   return NULL;
+}
+
+/**
+ * drm_gpuvm_bo_find() - find the _gpuvm_bo for the given
+ * _gpuvm and _gem_object
+ * @gpuvm: The _gpuvm the @obj is mapped in.
+ * @obj: The _gem_object being mapped in the @gpuvm.
+ *
+ * Find the _gpuvm_bo representing the combination of the given
+ * _gpuvm and _gem_object. If found, increases the reference
+ * count of the _gpuvm_bo accordingly.
+ *
+ * Returns: a pointer to the _gpuvm_bo on success, NULL on failure
+ */
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
+ struct drm_gem_object *obj)
+{
+   struct drm_gpuvm_bo *vm_bo = __drm_gpuvm_bo_find(gpuvm, obj);
+
+   return vm_bo ? drm_gpuvm_bo_get(vm_bo) : NULL;
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_find);
+
+/**
+ * drm_gpuvm_bo_obtain() - obtains and instance of the _gpuvm_bo for the
+ * given _gpuvm and _gem_object
+ * @gpuvm: The _gpuvm the @obj is mapped in.
+ * @obj: The _gem_object being mapped in the @gpuvm.
+ *
+ * Find the _gpuvm_bo representing the combination of the given
+ * _gpuvm and _gem_object. If found, increases the reference
+ * count of the _gpuvm_bo accordingly. If not found, allocates a new
+ * _gpuvm_bo.
+ *
+ * Returns: a pointer to the _gpuvm_bo on success, an ERR_PTR on failure
+ */
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_obtain(struct drm_gpuvm *gpuvm,
+   struct drm_gem_object *obj)
+{
+   struct drm_gpuvm_bo *vm_bo;
+
+   vm_bo = drm_gpuvm_bo_find(gpuvm, obj);
+   if (vm_bo)
+   return

[PATCH drm-misc-next v2 4/7] drm/gpuvm: common dma-resv per struct drm_gpuvm

Provide a common dma-resv for GEM objects not being used outside of this
GPU-VM. This is used in a subsequent patch to generalize dma-resv,
external and evicted object handling and GEM validation.

Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/drm_gpuva_mgr.c| 10 --
 drivers/gpu/drm/nouveau/nouveau_uvmm.c |  2 +-
 include/drm/drm_gpuva_mgr.h| 15 ++-
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c
index 0d9f15f50454..838277794990 100644
--- a/drivers/gpu/drm/drm_gpuva_mgr.c
+++ b/drivers/gpu/drm/drm_gpuva_mgr.c
@@ -655,6 +655,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
 /**
  * drm_gpuvm_init() - initialize a _gpuvm
  * @gpuvm: pointer to the _gpuvm to initialize
+ * @drm: the drivers _device
  * @name: the name of the GPU VA space
  * @start_offset: the start offset of the GPU VA space
  * @range: the size of the GPU VA space
@@ -668,7 +669,7 @@ drm_gpuva_range_valid(struct drm_gpuvm *gpuvm,
  *  is expected to be managed by the surrounding driver structures.
  */
 void
-drm_gpuvm_init(struct drm_gpuvm *gpuvm,
+drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
   const char *name,
   u64 start_offset, u64 range,
   u64 reserve_offset, u64 reserve_range,
@@ -694,6 +695,9 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm,
 reserve_range)))
__drm_gpuva_insert(gpuvm, >kernel_alloc_node);
}
+
+   drm_gem_private_object_init(drm, >d_obj, 0);
+   gpuvm->resv = gpuvm->d_obj.resv;
 }
 EXPORT_SYMBOL_GPL(drm_gpuvm_init);
 
@@ -713,7 +717,9 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
__drm_gpuva_remove(>kernel_alloc_node);
 
WARN(!RB_EMPTY_ROOT(>rb.tree.rb_root),
-"GPUVA tree is not empty, potentially leaking memory.");
+"GPUVA tree is not empty, potentially leaking memory.\n");
+
+   drm_gem_private_object_fini(>d_obj);
 }
 EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c 
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 4c47b2279674..21d5a06241ae 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1836,7 +1836,7 @@ nouveau_uvmm_init(struct nouveau_uvmm *uvmm, struct 
nouveau_cli *cli,
uvmm->kernel_managed_addr = kernel_managed_addr;
uvmm->kernel_managed_size = kernel_managed_size;
 
-   drm_gpuvm_init(>base, cli->name,
+   drm_gpuvm_init(>base, cli->drm->dev, cli->name,
   NOUVEAU_VA_SPACE_START,
   NOUVEAU_VA_SPACE_END,
   kernel_managed_addr, kernel_managed_size,
diff --git a/include/drm/drm_gpuva_mgr.h b/include/drm/drm_gpuva_mgr.h
index 1aa99a4d5665..e09e3e3ac5b2 100644
--- a/include/drm/drm_gpuva_mgr.h
+++ b/include/drm/drm_gpuva_mgr.h
@@ -240,9 +240,22 @@ struct drm_gpuvm {
 * @ops: _gpuvm_ops providing the split/merge steps to drivers
 */
const struct drm_gpuvm_ops *ops;
+
+   /**
+* @d_obj: Dummy GEM object; used internally to pass the GPU VMs
+* dma-resv to _exec.
+*/
+   struct drm_gem_object d_obj;
+
+   /**
+* @resv: the _resv for _gem_objects mapped in this GPU VA
+* space
+*/
+   struct dma_resv *resv;
 };
 
-void drm_gpuvm_init(struct drm_gpuvm *gpuvm, const char *name,
+void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
+   const char *name,
u64 start_offset, u64 range,
u64 reserve_offset, u64 reserve_range,
const struct drm_gpuvm_ops *ops);
-- 
2.41.0

[PATCH drm-misc-next v2 2/7] drm/gpuvm: rename struct drm_gpuva_manager to struct drm_gpuvm

Rename struct drm_gpuva_manager to struct drm_gpuvm including
corresponding functions. This way the GPUVA manager's structures align
very well with the documentation of VM_BIND [1] and VM_BIND locking [2].

It also provides a better foundation for the naming of data structures
and functions introduced for implementing a common dma-resv per GPU-VM
including tracking of external and evicted objects in subsequent
patches.

[1] Documentation/gpu/drm-vm-bind-async.rst
[2] Documentation/gpu/drm-vm-bind-locking.rst

Cc: Thomas Hellström 
Cc: Matthew Brost 
Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/drm_debugfs.c  |  14 +-
 drivers/gpu/drm/drm_gpuva_mgr.c| 352 -
 drivers/gpu/drm/nouveau/nouveau_exec.c |   2 +-
 drivers/gpu/drm/nouveau/nouveau_uvmm.c |  18 +-
 drivers/gpu/drm/nouveau/nouveau_uvmm.h |   4 +-
 include/drm/drm_debugfs.h  |   4 +-
 include/drm/drm_gpuva_mgr.h| 121 +
 7 files changed, 257 insertions(+), 258 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 34c7d1a580e3..7e1eab7da7a7 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -187,31 +187,31 @@ static const struct file_operations drm_debugfs_fops = {
 /**
  * drm_debugfs_gpuva_info - dump the given DRM GPU VA space
  * @m: pointer to the _file to write
- * @mgr: the _gpuva_manager representing the GPU VA space
+ * @gpuvm: the _gpuvm representing the GPU VA space
  *
  * Dumps the GPU VA mappings of a given DRM GPU VA manager.
  *
  * For each DRM GPU VA space drivers should call this function from their
  * _info_list's show callback.
  *
- * Returns: 0 on success, -ENODEV if the  is not initialized
+ * Returns: 0 on success, -ENODEV if the  is not initialized
  */
 int drm_debugfs_gpuva_info(struct seq_file *m,
-  struct drm_gpuva_manager *mgr)
+  struct drm_gpuvm *gpuvm)
 {
-   struct drm_gpuva *va, *kva = >kernel_alloc_node;
+   struct drm_gpuva *va, *kva = >kernel_alloc_node;
 
-   if (!mgr->name)
+   if (!gpuvm->name)
return -ENODEV;
 
seq_printf(m, "DRM GPU VA space (%s) [0x%016llx;0x%016llx]\n",
-  mgr->name, mgr->mm_start, mgr->mm_start + mgr->mm_range);
+  gpuvm->name, gpuvm->mm_start, gpuvm->mm_start + 
gpuvm->mm_range);
seq_printf(m, "Kernel reserved node [0x%016llx;0x%016llx]\n",
   kva->va.addr, kva->va.addr + kva->va.range);
seq_puts(m, "\n");
seq_puts(m, " VAs | start  | range  | end   
 | object | object offset\n");
seq_puts(m, 
"-\n");
-   drm_gpuva_for_each_va(va, mgr) {
+   drm_gpuvm_for_each_va(va, gpuvm) {
if (unlikely(va == kva))
continue;
 
diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c
index 6b5e12e142df..0d9f15f50454 100644
--- a/drivers/gpu/drm/drm_gpuva_mgr.c
+++ b/drivers/gpu/drm/drm_gpuva_mgr.c
@@ -33,8 +33,8 @@
 /**
  * DOC: Overview
  *
- * The DRM GPU VA Manager, represented by struct drm_gpuva_manager keeps track
- * of a GPU's virtual address (VA) space and manages the corresponding virtual
+ * The DRM GPU VA Manager, represented by struct drm_gpuvm keeps track of a
+ * GPU's virtual address (VA) space and manages the corresponding virtual
  * mappings represented by _gpuva objects. It also keeps track of the
  * mapping's backing _gem_object buffers.
  *
@@ -47,28 +47,28 @@
  * The GPU VA manager internally uses a rb-tree to manage the
  * _gpuva mappings within a GPU's virtual address space.
  *
- * The _gpuva_manager contains a special _gpuva representing the
+ * The _gpuvm structure contains a special _gpuva representing the
  * portion of VA space reserved by the kernel. This node is initialized 
together
  * with the GPU VA manager instance and removed when the GPU VA manager is
  * destroyed.
  *
- * In a typical application drivers would embed struct drm_gpuva_manager and
+ * In a typical application drivers would embed struct drm_gpuvm and
  * struct drm_gpuva within their own driver specific structures, there won't be
  * any memory allocations of its own nor memory allocations of _gpuva
  * entries.
  *
- * The data structures needed to store _gpuvas within the 
_gpuva_manager
- * are contained within struct drm_gpuva already. Hence, for inserting
- * _gpuva entries from within dma-fence signalling critical sections it is
- * enough to pre-allocate the _gpuva structures.
+ * The data structures needed to store _gpuvas within the _gpuvm are
+ * contained within struct drm_gpuva already. Hence, for inserting _gpuva
+ * entries from within dma-fence signalling critical sections it is enough to
+ * pre-allocate the _gpuva structures.
  */
 
 /**
  *

[PATCH drm-misc-next v2 1/7] drm: gpuva_mgr: allow building as module

Currently, the DRM GPUVA manager does not have any core dependencies
preventing a module build.

Also, new features from subsequent patches require helpers (namely
drm_exec) which can be built as module.

Signed-off-by: Danilo Krummrich 
---
 drivers/gpu/drm/Kconfig | 7 +++
 drivers/gpu/drm/Makefile| 2 +-
 drivers/gpu/drm/drm_gpuva_mgr.c | 3 +++
 drivers/gpu/drm/nouveau/Kconfig | 1 +
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index ab9ef1c20349..3f2577e10c07 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -216,6 +216,13 @@ config DRM_EXEC
help
  Execution context for command submissions
 
+config DRM_GPUVA_MGR
+   tristate
+   depends on DRM && DRM_EXEC
+   help
+ GPU-VM representation providing helpers to manage a GPUs virtual
+ address space
+
 config DRM_BUDDY
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 215e78e79125..5c10243d77fe 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -45,7 +45,6 @@ drm-y := \
drm_vblank.o \
drm_vblank_work.o \
drm_vma_manager.o \
-   drm_gpuva_mgr.o \
drm_writeback.o
 drm-$(CONFIG_DRM_LEGACY) += \
drm_agpsupport.o \
@@ -81,6 +80,7 @@ obj-$(CONFIG_DRM_PANEL_ORIENTATION_QUIRKS) += 
drm_panel_orientation_quirks.o
 #
 #
 obj-$(CONFIG_DRM_EXEC) += drm_exec.o
+obj-$(CONFIG_DRM_GPUVA_MGR) += drm_gpuva_mgr.o
 
 obj-$(CONFIG_DRM_BUDDY) += drm_buddy.o
 
diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c
index f86bfad74ff8..6b5e12e142df 100644
--- a/drivers/gpu/drm/drm_gpuva_mgr.c
+++ b/drivers/gpu/drm/drm_gpuva_mgr.c
@@ -1723,3 +1723,6 @@ drm_gpuva_ops_free(struct drm_gpuva_manager *mgr,
kfree(ops);
 }
 EXPORT_SYMBOL_GPL(drm_gpuva_ops_free);
+
+MODULE_DESCRIPTION("DRM GPUVA Manager");
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index c52e8096cca4..bf73f6b1ccad 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -11,6 +11,7 @@ config DRM_NOUVEAU
select DRM_TTM
select DRM_TTM_HELPER
select DRM_EXEC
+   select DRM_GPUVA_MGR
select DRM_SCHED
select I2C
select I2C_ALGOBIT
-- 
2.41.0

[PATCH drm-misc-next v2 0/7] [RFC] DRM GPUVA Manager GPU-VM features

So far the DRM GPUVA manager offers common infrastructure to track GPU VA
allocations and mappings, generically connect GPU VA mappings to their
backing buffers and perform more complex mapping operations on the GPU VA
space.

However, there are more design patterns commonly used by drivers, which
can potentially be generalized in order to make the DRM GPUVA manager
represent a basic GPU-VM implementation. In this context, this patch series
aims at generalizing the following elements.

1) Provide a common dma-resv for GEM objects not being used outside of
   this GPU-VM.

2) Provide tracking of external GEM objects (GEM objects which are
   shared with other GPU-VMs).

3) Provide functions to efficiently lock all GEM objects dma-resv the
   GPU-VM contains mappings of.

4) Provide tracking of evicted GEM objects the GPU-VM contains mappings
   of, such that validation of evicted GEM objects is accelerated.

5) Provide some convinience functions for common patterns.

The implementation introduces struct drm_gpuvm_bo, which serves as abstraction
combining a struct drm_gpuvm and struct drm_gem_object, similar to what
amdgpu does with struct amdgpu_bo_vm. While this adds a bit of complexity it
improves the efficiency of tracking external and evicted GEM objects.

This patch series also renames struct drm_gpuva_manager to struct drm_gpuvm
including corresponding functions. This way the GPUVA manager's structures align
better with the documentation of VM_BIND [1] and VM_BIND locking [2]. It also
provides a better foundation for the naming of data structures and functions
introduced for implementing the features of this patch series.

Rather than being designed as a "framework", the target is to make all
features appear as a collection of optional helper functions, such that
drivers are free to make use of the DRM GPUVA managers basic
functionality and opt-in for other features without setting any feature
flags, just by making use of the corresponding functions.

This patch series is also available at [3].

[1] Documentation/gpu/drm-vm-bind-async.rst
[2] Documentation/gpu/drm-vm-bind-locking.rst
[3] https://gitlab.freedesktop.org/nouvelles/kernel/-/commits/gpuvm-next

Changes in V2:
==
  - rename 'drm_gpuva_manager' -> 'drm_gpuvm' which generally leads to more
consistent naming
  - properly separate commits (introduce common dma-resv, drm_gpuvm_bo
abstraction, etc.)
  - remove maple tree for tracking external objects, use a list drm_gpuvm_bos
per drm_gpuvm instead
  - rework dma-resv locking helpers (Thomas)
  - add a locking helper for a given range of the VA space (Christian)
  - make the GPUVA manager buildable as module, rather than drm_exec
builtin (Christian)

Danilo Krummrich (7):
  drm: gpuva_mgr: allow building as module
  drm/gpuvm: rename struct drm_gpuva_manager to struct drm_gpuvm
  drm/nouveau: uvmm: rename 'umgr' to 'base'
  drm/gpuvm: common dma-resv per struct drm_gpuvm
  drm/gpuvm: add an abstraction for a VM / BO combination
  drm/gpuvm: generalize dma_resv/extobj handling and GEM validation
  drm/nouveau: GPUVM dma-resv/extobj handling, GEM validation

 drivers/gpu/drm/Kconfig   |   7 +
 drivers/gpu/drm/Makefile  |   2 +-
 drivers/gpu/drm/drm_debugfs.c |  14 +-
 drivers/gpu/drm/drm_gpuva_mgr.c   | 900 +-
 drivers/gpu/drm/nouveau/Kconfig   |   1 +
 drivers/gpu/drm/nouveau/nouveau_bo.c  |   4 +-
 drivers/gpu/drm/nouveau/nouveau_debugfs.c |   2 +-
 drivers/gpu/drm/nouveau/nouveau_exec.c|  52 +-
 drivers/gpu/drm/nouveau/nouveau_exec.h|   4 -
 drivers/gpu/drm/nouveau/nouveau_gem.c |   4 +-
 drivers/gpu/drm/nouveau/nouveau_sched.h   |   4 +-
 drivers/gpu/drm/nouveau/nouveau_uvmm.c| 216 --
 drivers/gpu/drm/nouveau/nouveau_uvmm.h|   6 +-
 include/drm/drm_debugfs.h |   4 +-
 include/drm/drm_gem.h |  48 +-
 include/drm/drm_gpuva_mgr.h   | 512 ++--
 16 files changed, 1375 insertions(+), 405 deletions(-)


base-commit: 6bd3d8da51ca1ec97c724016466606aec7739b9f
-- 
2.41.0

Re: [PATCH] drm: Rename drm_ioctl_flags() to eliminate duplicate declaration warning

2023-09-06 Thread Zack Rusin

On Thu, 2023-09-07 at 00:45 +0800, Juntong Deng wrote:
> There are 'enum drm_ioctl_flags' and 'bool drm_ioctl_flags(...)' with the
> same name, which is not a problem in C, but it can lead to
> 'WARNING: Duplicate C declaration' when generating documentation.
> 
> According to the purpose of the function, rename 'drm_ioctl_flags(...)' to
> 'drm_check_ioctl_flags(...)' to eliminate the warning.
> 
> Signed-off-by: Juntong Deng 
> ---
>  drivers/gpu/drm/drm_ioctl.c | 6 +++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
>  include/drm/drm_ioctl.h | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index f03ffbacfe9b..30699a0a10bc 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -911,7 +911,7 @@ long drm_ioctl(struct file *filp,
>  EXPORT_SYMBOL(drm_ioctl);
>  
>  /**
> - * drm_ioctl_flags - Check for core ioctl and return ioctl permission flags
> + * drm_check_ioctl_flags - Check for core ioctl and return ioctl permission 
> flags
>   * @nr: ioctl number
>   * @flags: where to return the ioctl permission flags
>   *
> @@ -922,7 +922,7 @@ EXPORT_SYMBOL(drm_ioctl);
>   * Returns:
>   * True if the @nr corresponds to a DRM core ioctl number, false otherwise.
>   */
> -bool drm_ioctl_flags(unsigned int nr, unsigned int *flags)
> +bool drm_check_ioctl_flags(unsigned int nr, unsigned int *flags)
>  {

Can we follow the namespace_action naming convention here? i.e.
drm_ioctl_flags_check instead. I find it a lot easier to look up/memorise the 
api if
naming is consistent.

z

Re: [PATCH v3 3/5] drm/msm: add trailing newlines to drm_dbg msgs

2023-09-06 Thread Abhinav Kumar


Hi Jim

On 9/6/2023 12:02 PM, Jim Cromie wrote:

By at least strong convention, a print-buffer's trailing newline says
"message complete, send it".  The exception (no TNL, followed by a call
to pr_cont) proves the general rule.

Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
1288 drm_dbg.  Clean up the remainders, in maintainer sized chunks.


May I know what 207, 1288 mean here? Is it the number of callers already 
having \n?


If so, this might be a big confusing as its subjective to the code-base 
you are referring to. So I will just stop with "Most DRM.debug calls 
already comport with this".




No functional changes.

Signed-off-by: Jim Cromie 
---
  drivers/gpu/drm/msm/msm_fb.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)



The change itself LGTM, hence

Reviewed-by: Abhinav Kumar

Re: [PATCH] drm/connector: document supported_colorspaces and DRM_MODE_COLORIMETRY_COUNT

2023-09-06 Thread Randy Dunlap

Hi,

On 9/6/23 11:19, Javier Carrasco wrote:
> The supported_colorspaces parameter was added to the following
> functions without documenting it:
> 
> drm_mode_create_dp_colorspace_property
> drm_mode_create_hdmi_colorspace_property
> 
> The missing descriptions generate warnings when compiling the
> documentation. Add the descriptions to document the
> supported_colorspaces parameter.
> 
> The drm_colorspace enum member DRM_MODE_COLORIMETRY_COUNT has been
> properly documented by moving the description out of the enum to the
> member description list to get rid of an additional warning and improve
> documentation clarity.
> 
> Signed-off-by: Javier Carrasco 
> ---
> The supported_colorspaces parameter was added to the following
> functions without documenting it:
> 
> drm_mode_create_dp_colorspace_property
> drm_mode_create_hdmi_colorspace_property
> 
> The missing descriptions generate warnings when compiling the
> documentation. Add the descriptions to document the
> supported_colorspaces parameter.
> 
> The drm_colorspace enum member DRM_MODE_COLORIMETRY_COUNT has been
> properly documented by moving the description out of the enum to the
> member description list to get rid of an additional warning and improve
> documentation clarity.
> ---
>  drivers/gpu/drm/drm_connector.c | 2 ++
>  include/drm/drm_connector.h | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index bf8371dc2a61..77bfe17dcf98 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c

These (2) are already fixed in linux-next.

> @@ -2203,6 +2203,7 @@ static int drm_mode_create_colorspace_property(struct 
> drm_connector *connector,
>  /**
>   * drm_mode_create_hdmi_colorspace_property - create hdmi colorspace property
>   * @connector: connector to create the Colorspace property on.
> + * @supported_colorspaces: colorspaces supported by the driver.
>   *
>   * Called by a driver the first time it's needed, must be attached to desired
>   * HDMI connectors.
> @@ -2227,6 +2228,7 @@ EXPORT_SYMBOL(drm_mode_create_hdmi_colorspace_property);
>  /**
>   * drm_mode_create_dp_colorspace_property - create dp colorspace property
>   * @connector: connector to create the Colorspace property on.
> + * @supported_colorspaces: colorspaces supported by the driver.
>   *
>   * Called by a driver the first time it's needed, must be attached to desired
>   * DP connectors.
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index d300fde6c1a4..18cf46e3478b 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h

This one still needs to be fixed/applied.
You can add:

Reviewed-by: Randy Dunlap 

Thanks.

> @@ -498,6 +498,8 @@ enum drm_privacy_screen_status {
>   *   ITU-R BT.601 colorimetry format
>   *   The DP spec does not say whether this is the 525 or the 625
>   *   line version.
> + * @DRM_MODE_COLORIMETRY_COUNT:
> + *   Not a valid value; merely used four counting
>   */
>  enum drm_colorspace {
>   /* For Default case, driver will set the colorspace */
> @@ -522,7 +524,6 @@ enum drm_colorspace {
>   DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED = 13,
>   DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT = 14,
>   DRM_MODE_COLORIMETRY_BT601_YCC  = 15,
> - /* not a valid value; merely used for counting */
>   DRM_MODE_COLORIMETRY_COUNT
>  };
>  
> 
> ---
> base-commit: 65d6e954e37872fd9afb5ef3fc0481bb3c2f20f4
> change-id: 20230906-topic-drm_connector_doc-42dae3ba43c6
> 
> Best regards,

-- 
~Randy

Re: [PATCH v2 07/34] drm/amd/display: explicitly define EOTF and inverse EOTF




On 2023-08-25 10:18, Melissa Wen wrote:
> On 08/22, Pekka Paalanen wrote:
>> On Thu, 10 Aug 2023 15:02:47 -0100
>> Melissa Wen  wrote:
>>
>>> Instead of relying on color block names to get the transfer function
>>> intention regarding encoding pixel's luminance, define supported
>>> Electro-Optical Transfer Functions (EOTFs) and inverse EOTFs, that
>>> includes pure gamma or standardized transfer functions.
>>>
>>> Suggested-by: Harry Wentland 
>>> Signed-off-by: Melissa Wen 
>>> ---
>>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 19 +++--
>>>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 69 +++
>>>  2 files changed, 67 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>>> index c749c9cb3d94..f6251ed89684 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
>>> @@ -718,14 +718,21 @@ extern const struct amdgpu_ip_block_version 
>>> dm_ip_block;
>>>  
>>>  enum amdgpu_transfer_function {
>>> AMDGPU_TRANSFER_FUNCTION_DEFAULT,
>>> -   AMDGPU_TRANSFER_FUNCTION_SRGB,
>>> -   AMDGPU_TRANSFER_FUNCTION_BT709,
>>> -   AMDGPU_TRANSFER_FUNCTION_PQ,
>>> +   AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_BT709_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_PQ_EOTF,
>>> AMDGPU_TRANSFER_FUNCTION_LINEAR,
>>> AMDGPU_TRANSFER_FUNCTION_UNITY,
>>> -   AMDGPU_TRANSFER_FUNCTION_GAMMA22,
>>> -   AMDGPU_TRANSFER_FUNCTION_GAMMA24,
>>> -   AMDGPU_TRANSFER_FUNCTION_GAMMA26,
>>> +   AMDGPU_TRANSFER_FUNCTION_GAMMA22_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF,
>>> +   AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF,
>>> +AMDGPU_TRANSFER_FUNCTION_COUNT
>>>  };
>>>  
>>>  struct dm_plane_state {
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>>> index 56ce008b9095..cc2187c0879a 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
>>> @@ -85,18 +85,59 @@ void amdgpu_dm_init_color_mod(void)
>>>  }
>>>  
>>>  #ifdef AMD_PRIVATE_COLOR
>>> -static const struct drm_prop_enum_list 
>>> amdgpu_transfer_function_enum_list[] = {
>>> -   { AMDGPU_TRANSFER_FUNCTION_DEFAULT, "Default" },
>>> -   { AMDGPU_TRANSFER_FUNCTION_SRGB, "sRGB" },
>>> -   { AMDGPU_TRANSFER_FUNCTION_BT709, "BT.709" },
>>> -   { AMDGPU_TRANSFER_FUNCTION_PQ, "PQ (Perceptual Quantizer)" },
>>> -   { AMDGPU_TRANSFER_FUNCTION_LINEAR, "Linear" },
>>> -   { AMDGPU_TRANSFER_FUNCTION_UNITY, "Unity" },
>>> -   { AMDGPU_TRANSFER_FUNCTION_GAMMA22, "Gamma 2.2" },
>>> -   { AMDGPU_TRANSFER_FUNCTION_GAMMA24, "Gamma 2.4" },
>>> -   { AMDGPU_TRANSFER_FUNCTION_GAMMA26, "Gamma 2.6" },
>>> +static const char * const
>>> +amdgpu_transfer_function_names[] = {
>>> +   [AMDGPU_TRANSFER_FUNCTION_DEFAULT]  = "Default",
>>> +   [AMDGPU_TRANSFER_FUNCTION_LINEAR]   = "Linear",
>>
>> Hi,
>>
>> if the below is identity, then what is linear? Is there a coefficient
>> (multiplier) somewhere? Offset?
>>
>>> +   [AMDGPU_TRANSFER_FUNCTION_UNITY]= "Unity",
>>
>> Should "Unity" be called "Identity"?
> 
> AFAIU, AMD treats Linear and Unity as the same: Identity. So, IIUC,
> indeed merging both as identity sounds the best approach.   

Agreed.

>>
>> Doesn't unity mean that the output is always 1.0 regardless of input?
>>
>>> +   [AMDGPU_TRANSFER_FUNCTION_SRGB_EOTF]= "sRGB EOTF",
>>> +   [AMDGPU_TRANSFER_FUNCTION_BT709_EOTF]   = "BT.709 EOTF",
>>
>> BT.709 says about "Overall opto-electronic transfer characteristics at
>> source":
>>
>>  In typical production practice the encoding function of image
>>  sources is adjusted so that the final picture has the desired
>>  look, as viewed on a reference monitor having the reference
>>  decoding function of Recommendation ITU-R BT.1886, in the
>>  reference viewing environment defined in Recommendation ITU-R
>>  BT.2035.
>>
>> IOW, typically people tweak the encoding function instead of using
>> BT.709 OETF as is, which means that inverting the BT.709 OETF produces
>> something slightly unknown. The note about BT.1886 means that that
>> something is also not quite how it's supposed to be turned into light.
>>
>> Should this enum item be "BT.709 inverse OETF" and respectively below a
>> "BT.709 OETF"?
>>
>> What curve does the hardware actually implement?
> 
> H.. I think I got confused in using OETF here since it's done within
> a camera. Looking at the coefficients used by AMD color

Re: [PATCH v2 2/5] fbdev: Replace fb_pgprotect() with fb_pgprot_device()

2023-09-06 Thread Arnd Bergmann

On Wed, Sep 6, 2023, at 10:35, Thomas Zimmermann wrote:
> Rename the fbdev mmap helper fb_pgprotect() to fb_pgprot_device().
> The helper sets VMA page-access flags for framebuffers in device I/O
> memory. The new name follows pgprot_device(), which does the same for
> arbitrary devices.
>
> Also clean up the helper's parameters and return value. Instead of
> the VMA instance, pass the individial parameters separately: existing
> page-access flags, the VMAs start and end addresses and the offset
> in the underlying device memory rsp file. Return the new page-access
> flags. These changes align fb_pgprot_device() closer with pgprot_device.
>
> Signed-off-by: Thomas Zimmermann 

This makes sense as a cleanup, but I'm not sure the new naming is helpful.

The 'pgprot_device' permissions are based on Arm's memory attributes,
which have slightly different behavior for "device", "uncached" and
"writecombine" mappings. I think simply calling this one pgprot_fb()
or fb_pgprot() would be less confusing, since depending on the architecture
it appears to give either uncached or writecombine mappings but not
"device" on the architectures where this is different.

  Arnd

Re: [V11 3/8] wifi: mac80211: Add support for WBRF features

2023-09-06 Thread Mario Limonciello


On 9/1/2023 09:32, Jeff Johnson wrote:

On 8/30/2023 11:20 PM, Evan Quan wrote:

To support the WBRF mechanism, Wifi adapters utilized in the system must


Since this is the first mention of WBRF in the core wireless code IMO 
you should indicate what this is an acronym for and briefly describe it

(or add a lore link).


A lot of information is captured in the cover letter and earlier 
commits.  I think you raise a good point that 10 years from now someone 
looking at random commits will have a hard time understanding what 
exactly WBRF stands for.


How about if we introduce a wbrf.rst somewhere in Documentation/ that 
explains the basic principles of how/why for it.  This Documentation 
patch could be the first in the series and then the commit message for 
wireless subsystem can tell people to look at that path for more 
information.




I'm wondering if WBRF is just a special case of frequency avoidance, and 
that more generic naming/terminology should be used in core wireless.
For example, I know there are vendor-specific solutions which allow 
Wi-Fi to avoid using channels which may conflict with cellular or 
BlueTooth, and those may benefit from a more generic




It seems to me that most vendor solutions that exist don't operate in 
the kernel code but usually in firmware based solutions, right?


I think to come up with a generic solution we need to first have a 
vendor that "wants" to participate in a generic solution to design it 
properly.



register the frequencies in use(or unregister those frequencies no longer
used) via the dedicated calls. So that, other drivers responding to the
frequencies can take proper actions to mitigate possible interference.

Co-developed-by: Mario Limonciello 
Signed-off-by: Mario Limonciello 
Co-developed-by: Evan Quan 
Signed-off-by: Evan Quan 
--
v1->v2:
   - place the new added member(`wbrf_supported`) in
 ieee80211_local(Johannes)
   - handle chandefs change scenario properly(Johannes)
   - some minor fixes around code sharing and possible invalid input
 checks(Johannes)
v2->v3:
   - drop unnecessary input checks and intermediate APIs(Mario)
   - Separate some mac80211 common code(Mario, Johannes)
v3->v4:
   - some minor fixes around return values(Johannes)
v9->v10:
   - get ranges_in->num_of_ranges set and passed in(Johannes)
---
  include/linux/ieee80211.h  |   1 +
  net/mac80211/Makefile  |   2 +
  net/mac80211/chan.c    |   9 
  net/mac80211/ieee80211_i.h |   9 
  net/mac80211/main.c    |   2 +
  net/mac80211/wbrf.c    | 105 +
  6 files changed, 128 insertions(+)
  create mode 100644 net/mac80211/wbrf.c

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 4b998090898e..f995d06da87f 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -4335,6 +4335,7 @@ static inline int 
ieee80211_get_tdls_action(struct sk_buff *skb, u32 hdr_size)

  /* convert frequencies */
  #define MHZ_TO_KHZ(freq) ((freq) * 1000)
  #define KHZ_TO_MHZ(freq) ((freq) / 1000)
+#define KHZ_TO_HZ(freq)  ((freq) * 1000)
  #define PR_KHZ(f) KHZ_TO_MHZ(f), f % 1000
  #define KHZ_F "%d.%03d"
diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index b8de44da1fb8..d46c36f55fd3 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -65,4 +65,6 @@ rc80211_minstrel-$(CONFIG_MAC80211_DEBUGFS) += \
  mac80211-$(CONFIG_MAC80211_RC_MINSTREL) += $(rc80211_minstrel-y)
+mac80211-y += wbrf.o
+
  ccflags-y += -DDEBUG
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 68952752b599..458469c224ae 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -506,11 +506,16 @@ static void _ieee80211_change_chanctx(struct 
ieee80211_local *local,

  WARN_ON(!cfg80211_chandef_compatible(>conf.def, chandef));
+    ieee80211_remove_wbrf(local, >conf.def);
+
  ctx->conf.def = *chandef;
  /* check if min chanctx also changed */
  changed = IEEE80211_CHANCTX_CHANGE_WIDTH |
    _ieee80211_recalc_chanctx_min_def(local, ctx, rsvd_for);
+
+    ieee80211_add_wbrf(local, >conf.def);
+
  drv_change_chanctx(local, ctx, changed);
  if (!local->use_chanctx) {
@@ -668,6 +673,8 @@ static int ieee80211_add_chanctx(struct 
ieee80211_local *local,

  lockdep_assert_held(>mtx);
  lockdep_assert_held(>chanctx_mtx);
+    ieee80211_add_wbrf(local, >conf.def);
+
  if (!local->use_chanctx)
  local->hw.conf.radar_enabled = ctx->conf.radar_enabled;
@@ -748,6 +755,8 @@ static void ieee80211_del_chanctx(struct 
ieee80211_local *local,

  }
  ieee80211_recalc_idle(local);
+
+    ieee80211_remove_wbrf(local, >conf.def);
  }
  static void ieee80211_free_chanctx(struct ieee80211_local *local,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 91633a0b723e..719f2c892132 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1600,6 +1600,8 @@ struct ieee80211_local {
  /* extended capabilities

Re: [PATCH v2 00/34] drm/amd/display: add AMD driver-specific properties for color mgmt

On 2023-08-10 12:02, Melissa Wen wrote:
> Hi all,
> 
> Here is the next version of our work to enable AMD driver-specific color
> management properties [1][2]. This series is a collection of
> contributions from Joshua, Harry, and me to enhance the AMD KMS color
> pipeline for Steam Deck/SteamOS by exposing additional pre-blending and
> post-blending color capabilities from those available in the current DRM
> KMS API[3].
> 
> The userspace case here is Gamescope which is the compositor for
> SteamOS. Gamescope is already using these features to implement its
> color management pipeline [4].
> 
> In this version, I try to address all concerns shared in the previous
> one, i.e.:
> - Replace DRM_ by AMDGPU_ prefix for transfer function enumeration; 
> - Explicitly define EOTFs and inverse EOTFs and set props accordingly;
> - Document pre-defined transfer functions;
> - Remove misleading comments;
> - Remove post-blending/MPC shaper and 3D LUT support;
> - Move driver-specific property operations from amdgpu_display.c to
>   amdgpu_dm_color.c;
> - Reset planes if any color props change;
> - Nits/small fixes;
> 
> Bearing in mind the complexity of color concepts, I believe there is a
> high chance of some misunderstanding from my side when defining EOTFs
> and documenting pre-defined TFs. So, reviews are very important and
> welcome (thanks in advance). FWIW, I added Harry as a co-developer of
> this TF documentation since I based on his description of EOTF/inv_EOTF
> and previous documentation work [5]. Let me know if there is a better
> way for credits.
> 
> Two DC patches were already applied and, therefore, removed from the
> series. I added r-b according to previous feedback. We also add plane
> CTM to driver-specific properties. As a result, this is the updated list
> of all driver-specific color properties exposed by this series:
> 
> - plane degamma LUT and pre-defined TF;
> - plane HDR multiplier;
> - plane CTM 3x4;
> - plane shaper LUT and pre-defined TF;
> - plane 3D LUT;
> - plane blend LUT and pre-defined TF;
> - CRTC gamma pre-defined TF;
> 
> Remember you can find the AMD HW color capabilities documented here:
> https://dri.freedesktop.org/docs/drm/gpu/amdgpu/display/display-manager.html#color-management-properties
> 
> Worth mentioning that the pre-blending degamma block can use ROM curves
> for some pre-defined TFs, but the other blocks use the AMD color module
> to calculate this curve considering pre-defined coefficients.
> 
> We need changes on DC gamut remap matrix to support the plane and CRTC
> CTM on drivers that support both. I've sent a previous patch to apply
> these changes to all DCN3+ families [6]. Here I use the same changes but
> limited to DCN301. Just let me know if you prefer the previous/expanded
> version.
> 
> Finally, this is the Linux/AMD color management API before and after
> blending with the driver-specific properties:
> 
> +--+
> |   PLANE  |
> |  |
> |  ++  |
> |  | AMD Degamma|  |
> |  ||  |
> |  | EOTF | 1D LUT  |  |
> |  ++---+  |
> |   |  |
> |  +v---+  |
> |  |AMD HDR |  |
> |  |Multiply|  |
> |  ++---+  |
> |   |  |
> |  +v---+  |
> |  |  AMD CTM (3x4) |  |
> |  ++---+  |
> |   |  |
> |  +v---+  |
> |  | AMD Shaper |  |
> |  ||  |
> |  | inv_EOTF | |  |
> |  | Custom 1D LUT  |  |
> |  ++---+  |
> |   |  |
> |  +v---+  |
> |  |   AMD 3D LUT   |  |
> |  |   17^3/12-bit  |  |
> |  ++---+  |
> |   |  |
> |  +v---+  |
> |  | AMD Blend  |  |
> |  ||  |
> |  | EOTF | 1D LUT  |  |
> |  ++---+  |
> |   |  |
> ++--v-++
> ||  Blending  ||
> ++--+-++
> |CRTC   |  |
> |   |  |
> |   +---v---+  |
> |   | DRM Degamma   |  |
> |   |   |  |
> |   | Custom 1D LUT |  |
> |   +---+---+  |
> |   |  |
> |   +---v---+  |
> |   | DRM CTM (3x3) |  |
> |   +---+---+  |
> |   |  |
> |   +---v---+  |
> |   | DRM Gamma |  |
> |   |   |  |
> |   | Custom 1D LUT |  |
> |   +---+  |
> |   | *AMD Gamma|  |
> |   |   inv_EOTF|  |
> |   +---+  |
> |  |
> +--+
> 
> Let me know your thoughts.
> 

Thanks again for your amazing work on this.

Patches 5, 6, 14, 16, and 24 are
Reviewed-by: Harry Wentland 

I left comments on the remaining unreviewed patches.

Harry

> Best Regards,
> 
> Melissa Wen
> 
> [1] https://lore.kernel.org/dri-devel/20230423141051.702990-1-m...@igalia.com
> [2] https://lore.kernel.org/dri-devel/20230523221520.3115570-1-m...@igalia.com
> [3] 
>

Re: [PATCH v2 11/34] drm/amd/display: add plane shaper LUT and TF driver-specific properties

On 2023-08-10 12:02, Melissa Wen wrote:
> On AMD HW, 3D LUT always assumes a preceding shaper 1D LUT used for
> delinearizing and/or normalizing the color space before applying a 3D
> LUT. Add pre-defined transfer function to enable delinearizing content
> with or without shaper LUT, where AMD color module calculates the
> resulted shaper curve. We apply an inverse EOTF to go from linear values
> to encoded values. If we are already in a non-linear space and/or don't
> need to normalize values, we can bypass shaper LUT with a linear
> transfer function that is also the default TF value.
> 

I think the color module will combine the TF and the custom 1D LUT
into the LUT that's actually programmed. We should spell out this
behavior in the comments below and in the patch description as it's
important for a userspace application to know.

The same applies to all other TF+LUT blocks.

Harry

> v2:
> - squash commits for shaper LUT and shaper TF
> - define inverse EOTF as supported shaper TFs
> 
> Signed-off-by: Melissa Wen 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  | 16 ++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 11 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 29 +
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 32 +++
>  4 files changed, 88 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 730a88236501..4fb164204ee6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -363,6 +363,22 @@ struct amdgpu_mode_info {
>* @plane_hdr_mult_property:
>*/
>   struct drm_property *plane_hdr_mult_property;
> + /**
> +  * @shaper_lut_property: Plane property to set pre-blending shaper LUT
> +  * that converts color content before 3D LUT.
> +  */
> + struct drm_property *plane_shaper_lut_property;
> + /**
> +  * @shaper_lut_size_property: Plane property for the size of
> +  * pre-blending shaper LUT as supported by the driver (read-only).
> +  */
> + struct drm_property *plane_shaper_lut_size_property;
> + /**
> +  * @plane_shaper_tf_property: Plane property to set a predefined
> +  * transfer function for pre-blending shaper (before applying 3D LUT)
> +  * with or without LUT.
> +  */
> + struct drm_property *plane_shaper_tf_property;
>   /**
>* @plane_lut3d_property: Plane property for gamma correction using a
>* 3D LUT (pre-blending).
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index deea90212e31..6b6c2980f0af 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -769,6 +769,17 @@ struct dm_plane_state {
>* S31.32 sign-magnitude.
>*/
>   __u64 hdr_mult;
> + /**
> +  * @shaper_lut: shaper lookup table blob. The blob (if not NULL) is an
> +  * array of  drm_color_lut.
> +  */
> + struct drm_property_blob *shaper_lut;
> + /**
> +  * @shaper_tf:
> +  *
> +  * Predefined transfer function to delinearize color space.
> +  */
> + enum amdgpu_transfer_function shaper_tf;
>   /**
>* @lut3d: 3D lookup table blob. The blob (if not NULL) is an array of
>*  drm_color_lut.
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 7e6d4df99a0c..fbcee717bf0a 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -151,6 +151,14 @@ static const u32 amdgpu_eotf =
>   BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_EOTF) |
>   BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_EOTF);
>  
> +static const u32 amdgpu_inv_eotf =
> + BIT(AMDGPU_TRANSFER_FUNCTION_SRGB_INV_EOTF) |
> + BIT(AMDGPU_TRANSFER_FUNCTION_BT709_INV_EOTF) |
> + BIT(AMDGPU_TRANSFER_FUNCTION_PQ_INV_EOTF) |
> + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA22_INV_EOTF) |
> + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA24_INV_EOTF) |
> + BIT(AMDGPU_TRANSFER_FUNCTION_GAMMA26_INV_EOTF);
> +
>  static struct drm_property *
>  amdgpu_create_tf_property(struct drm_device *dev,
> const char *name,
> @@ -209,6 +217,27 @@ amdgpu_dm_create_color_properties(struct amdgpu_device 
> *adev)
>   return -ENOMEM;
>   adev->mode_info.plane_hdr_mult_property = prop;
>  
> + prop = drm_property_create(adev_to_drm(adev),
> +DRM_MODE_PROP_BLOB,
> +"AMD_PLANE_SHAPER_LUT", 0);
> + if (!prop)
> + return -ENOMEM;
> + adev->mode_info.plane_shaper_lut_property = prop;
> +
> + prop = drm_property_create_range(adev_to_drm(adev),
> +

Re: [PATCH v2 10/34] drm/amd/display: add plane 3D LUT driver-specific properties




On 2023-08-10 12:02, Melissa Wen wrote:
> Add 3D LUT property for plane gamma correction using a 3D lookup table.
> Since a 3D LUT has a limited number of entries in each dimension we want
> to use them in an optimal fashion. This means using the 3D LUT in a
> colorspace that is optimized for human vision, such as sRGB, PQ, or
> another non-linear space. Therefore, userpace may need one 1D LUT
> (shaper) before it to delinearize content and another 1D LUT after 3D
> LUT (blend) to linearize content again for blending. The next patches
> add these 1D LUTs to the plane color mgmt pipeline.
> 
> Signed-off-by: Melissa Wen 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  | 10 
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  9 
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 14 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 23 +++
>  4 files changed, 56 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 66bae0eed80c..730a88236501 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -363,6 +363,16 @@ struct amdgpu_mode_info {
>* @plane_hdr_mult_property:
>*/
>   struct drm_property *plane_hdr_mult_property;
> + /**
> +  * @plane_lut3d_property: Plane property for gamma correction using a
> +  * 3D LUT (pre-blending).
> +  */

I think we'll want to describe how the 3DLUT entries are laid out.
Something that describes how userspace should fill it, like
gamescope does for example:
https://github.com/ValveSoftware/gamescope/blob/7108880ed80b68c21750369e2ac9b7315fecf264/src/color_helpers.cpp#L302

Something like: a three-dimensional array, with each dimension
having a size of the cubed root of lut3d_size, blue being the
outermost dimension, red the innermost.


> + struct drm_property *plane_lut3d_property;
> + /**
> +  * @plane_degamma_lut_size_property: Plane property to define the max
> +  * size of 3D LUT as supported by the driver (read-only).
> +  */

We should probably document that the size of the 3DLUT should
be the size of one dimension cubed, or that the cubed root of
the LUT size gives the size per dimension.

Harry

> + struct drm_property *plane_lut3d_size_property;
>  };
>  
>  #define AMDGPU_MAX_BL_LEVEL 0xFF
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 44f17ac11a5f..deea90212e31 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -769,6 +769,11 @@ struct dm_plane_state {
>* S31.32 sign-magnitude.
>*/
>   __u64 hdr_mult;
> + /**
> +  * @lut3d: 3D lookup table blob. The blob (if not NULL) is an array of
> +  *  drm_color_lut.
> +  */
> + struct drm_property_blob *lut3d;
>  };
>  
>  struct dm_crtc_state {
> @@ -854,6 +859,10 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector 
> *connector,
>  
>  void amdgpu_dm_trigger_timing_sync(struct drm_device *dev);
>  
> +/* 3D LUT max size is 17x17x17 */
> +#define MAX_COLOR_3DLUT_ENTRIES 4913
> +#define MAX_COLOR_3DLUT_BITDEPTH 12
> +/* 1D LUT size */
>  #define MAX_COLOR_LUT_ENTRIES 4096
>  /* Legacy gamm LUT users such as X doesn't like large LUT sizes */
>  #define MAX_COLOR_LEGACY_LUT_ENTRIES 256
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index b891aaf5f7c1..7e6d4df99a0c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -209,6 +209,20 @@ amdgpu_dm_create_color_properties(struct amdgpu_device 
> *adev)
>   return -ENOMEM;
>   adev->mode_info.plane_hdr_mult_property = prop;
>  
> + prop = drm_property_create(adev_to_drm(adev),
> +DRM_MODE_PROP_BLOB,
> +"AMD_PLANE_LUT3D", 0);
> + if (!prop)
> + return -ENOMEM;
> + adev->mode_info.plane_lut3d_property = prop;
> +
> + prop = drm_property_create_range(adev_to_drm(adev),
> +  DRM_MODE_PROP_IMMUTABLE,
> +  "AMD_PLANE_LUT3D_SIZE", 0, UINT_MAX);
> + if (!prop)
> + return -ENOMEM;
> + adev->mode_info.plane_lut3d_size_property = prop;
> +
>   return 0;
>  }
>  #endif
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index ab7f0332c431..882391f7add6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1353,6 +1353,8 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
>  
>   if

Re: [RFC, drm-misc-next v4 0/9] PCI/VGA: Allowing the user to select the primary video adapter at boot time

2023-09-06 Thread Alex Williamson

On Wed, 6 Sep 2023 11:51:59 +0800
Sui Jingfeng  wrote:

> Hi,
> 
> 
> On 2023/9/5 22:52, Alex Williamson wrote:
> > On Tue,  5 Sep 2023 03:57:15 +0800
> > Sui Jingfeng  wrote:
> >  
> >> From: Sui Jingfeng 
> >>
> >> On a machine with multiple GPUs, a Linux user has no control over which
> >> one is primary at boot time. This series tries to solve above mentioned
> >> problem by introduced the ->be_primary() function stub. The specific
> >> device drivers can provide an implementation to hook up with this stub by
> >> calling the vga_client_register() function.
> >>
> >> Once the driver bound the device successfully, VGAARB will call back to
> >> the device driver. To query if the device drivers want to be primary or
> >> not. Device drivers can just pass NULL if have no such needs.
> >>
> >> Please note that:
> >>
> >> 1) The ARM64, Loongarch, Mips servers have a lot PCIe slot, and I would
> >> like to mount at least three video cards.
> >>
> >> 2) Typically, those non-86 machines don't have a good UEFI firmware
> >> support, which doesn't support select primary GPU as firmware stage.
> >> Even on x86, there are old UEFI firmwares which already made undesired
> >> decision for you.
> >>
> >> 3) This series is attempt to solve the remain problems at the driver level,
> >> while another series[1] of me is target to solve the majority of the
> >> problems at device level.
> >>
> >> Tested (limited) on x86 with four video card mounted, Intel UHD Graphics
> >> 630 is the default boot VGA, successfully override by ast2400 with
> >> ast.modeset=10 append at the kernel cmd line.
> >>
> >> $ lspci | grep VGA
> >>
> >>   00:02.0 VGA compatible controller: Intel Corporation CoffeeLake-S GT2 
> >> [UHD Graphics 630]  
> > In all my previous experiments with VGA routing and IGD I found that
> > IGD can't actually release VGA routing and Intel confirmed the hardware
> > doesn't have the ability to do so.  
> 
> Which model of the IGD you are using? even for the IGD in Atom D2550,
> the legacy 128KB VGA memory range can be tuned to be mapped to IGD
> or to the DMI Interface. See the 1.7.3.2 section of the N2000 datasheet[1].

I believe it's the VGA I/O that can't be disabled, there's no means to
do so other than the I/O enable bit in the command register and iirc
the driver depends on this for other features.  The history of this is
pretty old, but here are some links:

https://lore.kernel.org/all/1376486637.31494.19.ca...@ul30vt.home/
https://bbs.archlinux.org/viewtopic.php?pid=1400212#p1400212
https://lore.kernel.org/all/20130815223917.27890.28003.st...@bling.home/
https://lore.kernel.org/all/20130824144701.23370.42110.st...@bling.home/
https://lore.kernel.org/all/20140509201655.2849.97478.st...@bling.home/

I think the issue was that i915 doesn't claim to the VGA arbiter to be
controlling legacy VGA ranges, but in fact the hardware does claim
those ranges.  We can "fix" i915 to report that VGA MMIO space is
owned and can be controlled, but then Xorg likely sees multiple VGA
arbiter clients and disables DRI because it wants to mmap VGA MMIO
space.

Therefore unless something has changed in the past 10yrs, i915 owns but
does not advertise ownership of the VGA address spaces and therefore
the arbiter can't and doesn't know to change VGA routing to enable a
"be_primary" path to another device.
 
> If a specific model of Intel has a bug in the VGA routing hardware logic unit,
> I would like to ignore it. Or switch to the UEFI firmware on such hardware.

That's a convenient and impractical approach.  I expect all Intel HD
graphics has this issue.  Unknown for Xe.

> It is the hardware engineer's responsibility, I will not worry about it.

We often need to deal with broken hardware in the kernel.

> Thanks for you tell this.
> 
> [1] 
> https://www.intel.com/content/dam/doc/datasheet/atom-d2000-n2000-vol-2-datasheet.pdf
> 
> 
> >   It will always be primary from a
> > VGA routing perspective.  Was this actually tested with non-UEFI?  
> 
> 
> As you already said, the generous Intel already have confirmed that the 
> hardware defect.
> So probably this is a good chance to switch to UEFI to solve the problem. 
> Then, no
> testing for legacy is needed.

Then why are we hacking on VGA arbitration in this series at all?

> > I suspect it might only work in UEFI mode where we probably don't
> > actually have a dependency on VGA routing.  This is essentially why
> > vfio requires UEFI ROMs when assigning GPUs to VMs, VGA routing is too
> > broken to use on Intel systems with IGD.  Thanks,  
> 
> Thanks for you tell me this.
> 
> To be honest, I have only tested my patch on machines with UEFI firmware.
> Since UEFI because the main stream, but if this patch is really useful for
> majority machine, I'm satisfied. The results is not too bad.

This looks like a pretty significant scoping issue if you're proposing
changes to the VGA arbiter which specifically handles the routing of
legacy VGA address spaces

Re: [PATCH v2 01/34] drm/amd/display: fix segment distribution for linear LUTs

On 2023-08-10 12:02, Melissa Wen wrote:
> From: Harry Wentland 
> 
> The region and segment calculation was incapable of dealing
> with regions of more than 16 segments. We first fix this.
> 
> Now that we can support regions up to 256 elements we can
> define a better segment distribution for near-linear LUTs
> for our maximum of 256 HW-supported points.
> 
> With these changes an "identity" LUT looks visually
> indistinguishable from bypass and allows us to use
> our 3DLUT.
> 

Have you had a chance to test whether this patch makes a
difference? I haven't had the time yet.

Harry

> Signed-off-by: Harry Wentland 
> Signed-off-by: Melissa Wen 
> ---
>  .../amd/display/dc/dcn10/dcn10_cm_common.c| 93 +++
>  1 file changed, 75 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c 
> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> index 3538973bd0c6..04b2e04b68f3 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c
> @@ -349,20 +349,37 @@ bool cm_helper_translate_curve_to_hw_format(struct 
> dc_context *ctx,
>* segment is from 2^-10 to 2^1
>* There are less than 256 points, for optimization
>*/
> - seg_distr[0] = 3;
> - seg_distr[1] = 4;
> - seg_distr[2] = 4;
> - seg_distr[3] = 4;
> - seg_distr[4] = 4;
> - seg_distr[5] = 4;
> - seg_distr[6] = 4;
> - seg_distr[7] = 4;
> - seg_distr[8] = 4;
> - seg_distr[9] = 4;
> - seg_distr[10] = 1;
> + if (output_tf->tf == TRANSFER_FUNCTION_LINEAR) {
> + seg_distr[0] = 0; /* 2 */
> + seg_distr[1] = 1; /* 4 */
> + seg_distr[2] = 2; /* 4 */
> + seg_distr[3] = 3; /* 8 */
> + seg_distr[4] = 4; /* 16 */
> + seg_distr[5] = 5; /* 32 */
> + seg_distr[6] = 6; /* 64 */
> + seg_distr[7] = 7; /* 128 */
> +
> + region_start = -8;
> + region_end = 1;
> + } else {
> + seg_distr[0] = 3; /* 8 */
> + seg_distr[1] = 4; /* 16 */
> + seg_distr[2] = 4;
> + seg_distr[3] = 4;
> + seg_distr[4] = 4;
> + seg_distr[5] = 4;
> + seg_distr[6] = 4;
> + seg_distr[7] = 4;
> + seg_distr[8] = 4;
> + seg_distr[9] = 4;
> + seg_distr[10] = 1; /* 2 */
> + /* total = 8*16 + 8 + 64 + 2 = */
> +
> + region_start = -10;
> + region_end = 1;
> + }
> +
>  
> - region_start = -10;
> - region_end = 1;
>   }
>  
>   for (i = region_end - region_start; i < MAX_REGIONS_NUMBER ; i++)
> @@ -375,16 +392,56 @@ bool cm_helper_translate_curve_to_hw_format(struct 
> dc_context *ctx,
>  
>   j = 0;
>   for (k = 0; k < (region_end - region_start); k++) {
> - increment = NUMBER_SW_SEGMENTS / (1 << seg_distr[k]);
> + /*
> +  * We're using an ugly-ish hack here. Our HW allows for
> +  * 256 segments per region but SW_SEGMENTS is 16.
> +  * SW_SEGMENTS has some undocumented relationship to
> +  * the number of points in the tf_pts struct, which
> +  * is 512, unlike what's suggested TRANSFER_FUNC_POINTS.
> +  *
> +  * In order to work past this dilemma we'll scale our
> +  * increment by (1 << 4) and then do the inverse (1 >> 4)
> +  * when accessing the elements in tf_pts.
> +  *
> +  * TODO: find a better way using SW_SEGMENTS and
> +  *   TRANSFER_FUNC_POINTS definitions
> +  */
> + increment = (NUMBER_SW_SEGMENTS << 4) / (1 << seg_distr[k]);
>   start_index = (region_start + k + MAX_LOW_POINT) *
>   NUMBER_SW_SEGMENTS;
> - for (i = start_index; i < start_index + NUMBER_SW_SEGMENTS;
> + for (i = (start_index << 4); i < (start_index << 4) + 
> (NUMBER_SW_SEGMENTS << 4);
>   i += increment) {
> + struct fixed31_32 in_plus_one, in;
> + struct fixed31_32 value, red_value, green_value, 
> blue_value;
> + uint32_t t = i & 0xf;
> +
>   if (j == hw_points - 1)
>   break;
> - rgb_resulted[j].red = output_tf->tf_pts.red[i];
> - rgb_resulted[j].green = output_tf->tf_pts.green[i];
> - rgb_resulted[j].blue =

Re: [PATCH v6 03/20] dt-bindings: gpu: Add Imagination Technologies PowerVR/IMG GPU

2023-09-06 Thread Linus Walleij

On Wed, Sep 6, 2023 at 11:56 AM Sarah Walker  wrote:

> Add the device tree binding documentation for the IMG AXE GPU used in
> TI AM62 SoCs.
>
> Co-developed-by: Frank Binns 
> Signed-off-by: Frank Binns 
> Signed-off-by: Sarah Walker 
> ---
> Changes since v5:
> - Update compatible string & description to match marketing name
> - Remove unnecessary clock-names definition in ti,am62-gpu constraints
> - Document that GPU revision is discoverable

This looks good to me!
Reviewed-by: Linus Walleij 

Yours,
Linus Walleij

[PATCH v3 5/5] drm/Makefile: use correct ccflags-y syntax

Incorrect CFLAGS- usage failed to add -DDYNAMIC_DEBUG_MODULE when needed,
which broke builds with:

CONFIG_DRM_USE_DYNAMIC_DEBUG=Y
CONFIG_DYNAMIC_DEBUG_CORE=Y
CONFIG_DYNAMIC_DEBUG=N

Also add subdir-ccflags so that all drivers pick up the addition.

Fixes: 84ec67288c10 ("drm_print: wrap drm_*_dbg in dyndbg descriptor factory 
macro")
Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 7a09a89b493b..013cde886326 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -3,7 +3,8 @@
 # Makefile for the drm device driver.  This driver provides support for the
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
-CFLAGS-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
+ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG)+= 
-DDYNAMIC_DEBUG_MODULE
+subdir-ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
 
 drm-y := \
drm_aperture.o \
-- 
2.41.0

[PATCH v3 4/5] drm/vc4: add trailing newlines to drm_dbg msgs

By at least strong convention, a print-buffer's trailing newline says
"message complete, send it".  The exception (no TNL, followed by a call
to pr_cont) proves the general rule.

Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
1288 drm_dbg.  Clean up the remainders, in maintainer sized chunks.

No functional changes.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index bef9d45ef1df..959123759711 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -592,7 +592,7 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc, old_state);
struct drm_device *dev = crtc->dev;
 
-   drm_dbg(dev, "Disabling CRTC %s (%u) connected to Encoder %s (%u)",
+   drm_dbg(dev, "Disabling CRTC %s (%u) connected to Encoder %s (%u)\n",
crtc->name, crtc->base.id, encoder->name, encoder->base.id);
 
require_hvs_enabled(dev);
@@ -620,7 +620,7 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
int idx;
 
-   drm_dbg(dev, "Enabling CRTC %s (%u) connected to Encoder %s (%u)",
+   drm_dbg(dev, "Enabling CRTC %s (%u) connected to Encoder %s (%u)\n",
crtc->name, crtc->base.id, encoder->name, encoder->base.id);
 
if (!drm_dev_enter(dev, ))
-- 
2.41.0

[PATCH v3 3/5] drm/msm: add trailing newlines to drm_dbg msgs

By at least strong convention, a print-buffer's trailing newline says
"message complete, send it".  The exception (no TNL, followed by a call
to pr_cont) proves the general rule.

Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
1288 drm_dbg.  Clean up the remainders, in maintainer sized chunks.

No functional changes.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/msm/msm_fb.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c
index e3f61c39df69..88bb5fa23bb1 100644
--- a/drivers/gpu/drm/msm/msm_fb.c
+++ b/drivers/gpu/drm/msm/msm_fb.c
@@ -89,7 +89,7 @@ int msm_framebuffer_prepare(struct drm_framebuffer *fb,
 
for (i = 0; i < n; i++) {
ret = msm_gem_get_and_pin_iova(fb->obj[i], aspace, 
_fb->iova[i]);
-   drm_dbg_state(fb->dev, "FB[%u]: iova[%d]: %08llx (%d)",
+   drm_dbg_state(fb->dev, "FB[%u]: iova[%d]: %08llx (%d)\n",
  fb->base.id, i, msm_fb->iova[i], ret);
if (ret)
return ret;
@@ -176,9 +176,9 @@ static struct drm_framebuffer *msm_framebuffer_init(struct 
drm_device *dev,
const struct msm_format *format;
int ret, i, n;
 
-   drm_dbg_state(dev, "create framebuffer: mode_cmd=%p (%dx%d@%4.4s)",
-   mode_cmd, mode_cmd->width, mode_cmd->height,
-   (char *)_cmd->pixel_format);
+   drm_dbg_state(dev, "create framebuffer: mode_cmd=%p (%dx%d@%4.4s)\n",
+ mode_cmd, mode_cmd->width, mode_cmd->height,
+ (char *)_cmd->pixel_format);
 
n = info->num_planes;
format = kms->funcs->get_format(kms, mode_cmd->pixel_format,
@@ -232,7 +232,7 @@ static struct drm_framebuffer *msm_framebuffer_init(struct 
drm_device *dev,
 
refcount_set(_fb->dirtyfb, 1);
 
-   drm_dbg_state(dev, "create: FB ID: %d (%p)", fb->base.id, fb);
+   drm_dbg_state(dev, "create: FB ID: %d (%p)\n", fb->base.id, fb);
 
return fb;
 
-- 
2.41.0

[PATCH v3 1/5] drm/connector: add trailing newlines to drm_dbg msgs

By at least strong convention, a print-buffer's trailing newline says
"message complete, send it".  The exception (no TNL, followed by a call
to pr_cont) proves the general rule.

Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
1288 drm_dbg.  Clean up the remainders, in maintainer sized chunks.

No functional changes.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/drm_connector.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index f28725736237..14020585bdc0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2925,7 +2925,9 @@ int drm_mode_getconnector(struct drm_device *dev, void 
*data,
 dev->mode_config.max_width,
 
dev->mode_config.max_height);
else
-   drm_dbg_kms(dev, "User-space requested a forced probe 
on [CONNECTOR:%d:%s] but is not the DRM master, demoting to read-only probe",
+   drm_dbg_kms(dev,
+   "User-space requested a forced probe on 
[CONNECTOR:%d:%s] "
+   "but is not the DRM master, demoting to 
read-only probe\n",
connector->base.id, connector->name);
}
 
-- 
2.41.0

[PATCH v3 2/5] drm/kmb: add trailing newlines to drm_dbg msgs

By at least strong convention, a print-buffer's trailing newline says
"message complete, send it".  The exception (no TNL, followed by a call
to pr_cont) proves the general rule.

Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
1288 drm_dbg.  Clean up the remainders, in maintainer sized chunks.

No functional changes.

Signed-off-by: Jim Cromie 
---
 drivers/gpu/drm/kmb/kmb_crtc.c  | 10 +-
 drivers/gpu/drm/kmb/kmb_plane.c |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c
index 647872f65bff..a58baf25322d 100644
--- a/drivers/gpu/drm/kmb/kmb_crtc.c
+++ b/drivers/gpu/drm/kmb/kmb_crtc.c
@@ -94,7 +94,7 @@ static void kmb_crtc_set_mode(struct drm_crtc *crtc,
vm.hback_porch = 0;
vm.hsync_len = 28;
 
-   drm_dbg(dev, "%s : %dactive height= %d vbp=%d vfp=%d vsync-w=%d 
h-active=%d h-bp=%d h-fp=%d hsync-l=%d",
+   drm_dbg(dev, "%s : %dactive height= %d vbp=%d vfp=%d vsync-w=%d 
h-active=%d h-bp=%d h-fp=%d hsync-l=%d\n",
__func__, __LINE__,
m->crtc_vdisplay, vm.vback_porch, vm.vfront_porch,
vm.vsync_len, m->crtc_hdisplay, vm.hback_porch,
@@ -194,24 +194,24 @@ static enum drm_mode_status
int vfp = mode->vsync_start - mode->vdisplay;
 
if (mode->vdisplay < KMB_CRTC_MAX_HEIGHT) {
-   drm_dbg(dev, "height = %d less than %d",
+   drm_dbg(dev, "height = %d less than %d\n",
mode->vdisplay, KMB_CRTC_MAX_HEIGHT);
return MODE_BAD_VVALUE;
}
if (mode->hdisplay < KMB_CRTC_MAX_WIDTH) {
-   drm_dbg(dev, "width = %d less than %d",
+   drm_dbg(dev, "width = %d less than %d\n",
mode->hdisplay, KMB_CRTC_MAX_WIDTH);
return MODE_BAD_HVALUE;
}
refresh = drm_mode_vrefresh(mode);
if (refresh < KMB_MIN_VREFRESH || refresh > KMB_MAX_VREFRESH) {
-   drm_dbg(dev, "refresh = %d less than %d or greater than %d",
+   drm_dbg(dev, "refresh = %d less than %d or greater than %d\n",
refresh, KMB_MIN_VREFRESH, KMB_MAX_VREFRESH);
return MODE_BAD;
}
 
if (vfp < KMB_CRTC_MIN_VFP) {
-   drm_dbg(dev, "vfp = %d less than %d", vfp, KMB_CRTC_MIN_VFP);
+   drm_dbg(dev, "vfp = %d less than %d\n", vfp, KMB_CRTC_MIN_VFP);
return MODE_BAD;
}
 
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c
index 9e0562aa2bcb..308bd1cb50c8 100644
--- a/drivers/gpu/drm/kmb/kmb_plane.c
+++ b/drivers/gpu/drm/kmb/kmb_plane.c
@@ -78,7 +78,7 @@ static unsigned int check_pixel_format(struct drm_plane 
*plane, u32 format)
 * plane configuration is not supported.
 */
if (init_disp_cfg.format && init_disp_cfg.format != format) {
-   drm_dbg(>drm, "Cannot change format after initial plane 
configuration");
+   drm_dbg(>drm, "Cannot change format after initial plane 
configuration\n");
return -EINVAL;
}
for (i = 0; i < plane->format_count; i++) {
@@ -124,7 +124,7 @@ static int kmb_plane_atomic_check(struct drm_plane *plane,
if ((init_disp_cfg.width && init_disp_cfg.height) &&
(init_disp_cfg.width != fb->width ||
init_disp_cfg.height != fb->height)) {
-   drm_dbg(>drm, "Cannot change plane height or width after 
initial configuration");
+   drm_dbg(>drm, "Cannot change plane height or width after 
initial configuration\n");
return -EINVAL;
}
can_position = (plane->type == DRM_PLANE_TYPE_OVERLAY);
@@ -375,7 +375,7 @@ static void kmb_plane_atomic_update(struct drm_plane *plane,
spin_lock_irq(>irq_lock);
if (kmb->kmb_under_flow || kmb->kmb_flush_done) {
spin_unlock_irq(>irq_lock);
-   drm_dbg(>drm, "plane_update:underflow returning");
+   drm_dbg(>drm, "plane_update:underflow returning\n");
return;
}
spin_unlock_irq(>irq_lock);
-- 
2.41.0

[PATCH v3 0/5] drm/drm_dbg: add trailing newlines where missing

By at least strong convention, a print-buffer's trailing newline says
"message complete, send it".  The exception (no TNL, followed by a call
to pr_cont) proves the general rule.

Most DRM.debug calls already comport with this rule/convention:
207 DRM_DEV_DEBUG, 1288 drm_dbg.  Clean up the remainders, in
maintainer sized chunks.

V3: adds proper "drm/:" to subject, as suggested by Rodrigo.
drops drm/i915: already applied by Rodrigo.

Jim Cromie (5):
  drm/connector: add trailing newlines to drm_dbg msgs
  drm/kmb: add trailing newlines to drm_dbg msgs
  drm/msm: add trailing newlines to drm_dbg msgs
  drm/vc4: add trailing newlines to drm_dbg msgs
  drm/Makefile: use correct ccflags-y syntax

 drivers/gpu/drm/Makefile|  3 ++-
 drivers/gpu/drm/drm_connector.c |  4 +++-
 drivers/gpu/drm/kmb/kmb_crtc.c  | 10 +-
 drivers/gpu/drm/kmb/kmb_plane.c |  6 +++---
 drivers/gpu/drm/msm/msm_fb.c| 10 +-
 drivers/gpu/drm/vc4/vc4_crtc.c  |  4 ++--
 6 files changed, 20 insertions(+), 17 deletions(-)

-- 
2.41.0

Re: [PATCH v5] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset

2023-09-06 Thread John Harrison


On 9/6/2023 02:17, Andi Shyti wrote:

Hi John,


 static void guc_cancel_busyness_worker(struct intel_guc *guc)
 {
-   cancel_delayed_work_sync(>timestamp.work);
+   /*
+* When intel_gt_reset was called, task will hold a lock.
+* To cacel delayed work here, the _sync version will also acquire a 
lock, which might
+* trigger the possible cirular locking dependency warning.
+* Check the reset_in_progress flag, call async verion if reset is in 
progress.
+*/

This needs to explain in much more detail what is going on and why it is not
a problem. E.g.:

  The busyness worker needs to be cancelled. In general that means
  using the synchronous cancel version to ensure that an in-progress
  worker will not keep executing beyond whatever is happening that
  needs the cancel. E.g. suspend, driver unload, etc. However, in the
  case of a reset, the synchronous version is not required and can
  trigger a false deadlock detection warning.

  The business worker takes the reset mutex to protect against resets
  interfering with it. However, it does a trylock and bails out if the
  reset lock is already acquired. Thus there is no actual deadlock or
  other concern with the worker running concurrently with a reset. So
  an asynchronous cancel is safe in the case of a reset rather than a
  driver unload or suspend type operation. On the other hand, if the
  cancel_sync version is used when a reset is in progress then the
  mutex deadlock detection sees the mutex being acquired through
  multiple paths and complains.

  So just don't bother. That keeps the detection code happy and is
  safe because of the trylock code described above.

So why do we even need to cancel anything if it doesn't do anything while
the reset is in progress?

It still needs to be cancelled. The worker only aborts if it is actively
executing concurrently with the reset. It might not start to execute until
after the reset has completed. And there is presumably a reason why the
cancel is being called, a reason not necessarily related to resets at all.
Leaving the worker to run arbitrarily after the driver is expecting it to be
stopped will lead to much worse things than a fake lockdep splat, e.g. a use
after free pointer deref.

I was actually thinking why not leave things as they are and just
disable lockdep from CI. This doesn't look like a relevant report
to me.

Andi

Disable lockdep? The whole of lockdep? We absolutely do not want to disable
an extremely important deadlock testing infrastructure in our test
framework. That would be defeating the whole point of CI.

Potentially we could annotate this one particular scenario to suppress this
one particular error.  But it seems simpler and safer to just update the
code to not hit that scenario in the first place.

yes... lockdep is a debug tool and might provide false reports...
We need to have a great willingness to start fixing and hunting
debug lockdep's false positives (like this one, for instance).
That is how lockdep works. It's like a compiler warning. You have to fix 
them even if you think they don't matter. Because otherwise, when 
someone tries to turn warnings on, they drown in a sea of other people's 
unrelated garbage that they did not bother to fix. If lockdep is to be 
of any use at all then it must be run regularly as part of a CI type 
system and any issues it finds must be fixed up by the developer's that 
own the relevant code. Where fixing means either fixing genuine bugs, 
re-working the code to not hit a false positive or annotating the code 
to explain to lockdep why it is a safe operation.




It's even more annoying to reduce our CI pass rates, especially
when in BAT tests, with such false deadlocks.
Maybe. But it is even more annoying when you have a genuine locking 
issue that you don't notice because you have disabled lockdep and just 
have some random hang issue that is impossible to reproduce or debug.




It's the developer's responsibility to test its code with
debug_lockdep and fix all the potential deadlocks and ignore the
false ones.
You seem to have this backwards. Developers are not expected to run 
every possible test on every possible platform in every possible 
configuration. That is the job of CI.


John.


I sent a patch for this[*] already.

Andi

[*] https://gitlab.freedesktop.org/gfx-ci/i915-infra/-/merge_requests/128

Re: [Intel-gfx] [PATCH v5] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset

2023-09-06 Thread John Harrison


On 9/5/2023 23:50, Daniel Vetter wrote:

On Mon, Aug 28, 2023 at 04:01:38PM -0700, John Harrison wrote:

On 8/23/2023 10:37, John Harrison wrote:

On 8/23/2023 09:00, Daniel Vetter wrote:

On Tue, Aug 22, 2023 at 11:53:24AM -0700, John Harrison wrote:

On 8/11/2023 11:20, Zhanjun Dong wrote:

This attempts to avoid circular locking dependency between
flush delayed
work and intel_gt_reset.
When intel_gt_reset was called, task will hold a lock.
To cacel delayed work here, the _sync version will also
acquire a lock,
which might trigger the possible cirular locking dependency warning.
When intel_gt_reset called, reset_in_progress flag will be
set, add code
to check the flag, call async verion if reset is in progress.

Signed-off-by: Zhanjun Dong
Cc: John Harrison
Cc: Andi Shyti
Cc: Daniel Vetter
---
    drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 11 ++-
    1 file changed, 10 insertions(+), 1 deletion(-)

diff --git
a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0e3ef1c65d2..600388c849f7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1359,7 +1359,16 @@ static void
guc_enable_busyness_worker(struct intel_guc *guc)
    static void guc_cancel_busyness_worker(struct intel_guc *guc)
    {
-    cancel_delayed_work_sync(>timestamp.work);
+    /*
+ * When intel_gt_reset was called, task will hold a lock.
+ * To cacel delayed work here, the _sync version will
also acquire a lock, which might
+ * trigger the possible cirular locking dependency warning.
+ * Check the reset_in_progress flag, call async verion
if reset is in progress.
+ */

This needs to explain in much more detail what is going on and
why it is not
a problem. E.g.:

     The busyness worker needs to be cancelled. In general that means
     using the synchronous cancel version to ensure that an in-progress
     worker will not keep executing beyond whatever is happening that
     needs the cancel. E.g. suspend, driver unload, etc. However, in the
     case of a reset, the synchronous version is not required and can
     trigger a false deadlock detection warning.

     The business worker takes the reset mutex to protect against resets
     interfering with it. However, it does a trylock and bails
out if the
     reset lock is already acquired. Thus there is no actual deadlock or
     other concern with the worker running concurrently with a reset. So
     an asynchronous cancel is safe in the case of a reset rather than a
     driver unload or suspend type operation. On the other hand, if the
     cancel_sync version is used when a reset is in progress then the
     mutex deadlock detection sees the mutex being acquired through
     multiple paths and complains.

     So just don't bother. That keeps the detection code happy and is
     safe because of the trylock code described above.

So why do we even need to cancel anything if it doesn't do anything
while
the reset is in progress?

It still needs to be cancelled. The worker only aborts if it is actively
executing concurrently with the reset. It might not start to execute
until after the reset has completed. And there is presumably a reason
why the cancel is being called, a reason not necessarily related to
resets at all. Leaving the worker to run arbitrarily after the driver is
expecting it to be stopped will lead to much worse things than a fake
lockdep splat, e.g. a use after free pointer deref.

John.

@Daniel Vetter - ping? Is this explanation sufficient? Are you okay with
this change now?

Sorry for the late reply, I'm constantly behind on mails :-/ Ping me on
irc next time around if I don't reply, that's quicker.

"presumably" isn't good enough for locking design. Either you know, and
can prove it all, or you shouldn't touch the code and its locking design
before you've figured this out.

Again, either this is a deadlock, race condition, or the cancel isn't
necessary. And this argument works in full generality. All this patch does
it replace the dealock with one of the other two, and that's not good
enough if you don't even know which one it is.

- if you need the cancel, you have a race condition

- if you don't have a race condition, you don't need the cancel
In the case of a reset in progress, we do not strictly need the cancel. 
The worker thread will take care of avoiding a deadlock by itself. But 
it is more efficient to do the cancel and avoid unnecessary code 
execution if possible. It is also more logically correct - the worker is 
being stopped, therefore we should cancel any pending execution of the 
worker.


In the case of a reset not being in progress, we absolutely do need the 
cancel as there are multiple race conditions.




- currently you have the deadlock

No, we do not. There is no deadlock.

The worker thread explicitly does a trylock and reschedules itself for 
later if it could not get the lock.

Re: [PATCH v2 34/34] drm/amd/display: Use 3x4 CTM for plane CTM




On 2023-08-10 12:03, Melissa Wen wrote:
> From: Joshua Ashton 
> 
> Signed-off-by: Joshua Ashton 
> Signed-off-by: Melissa Wen 
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 32 +--
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  2 +-
>  include/uapi/drm/drm_mode.h   |  8 +
>  3 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 7ff329101fd4..0a51af44efd5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -412,6 +412,32 @@ static void __drm_ctm_to_dc_matrix(const struct 
> drm_color_ctm *ctm,
>   }
>  }
>  
> +/**
> + * __drm_ctm2_to_dc_matrix - converts a DRM CTM2 to a DC CSC float matrix
> + * @ctm: DRM color transformation matrix
> + * @matrix: DC CSC float matrix
> + *
> + * The matrix needs to be a 3x4 (12 entry) matrix.
> + */
> +static void __drm_ctm2_to_dc_matrix(const struct drm_color_ctm2 *ctm,
> +struct fixed31_32 *matrix)
> +{
> + int i;
> +
> + /*
> +  * DRM gives a 3x3 matrix, but DC wants 3x4. Assuming we're operating
> +  * with homogeneous coordinates, augment the matrix with 0's.
> +  *

Left-over copy-paste comment. This version takes 3x4 as input param.

> +  * The format provided is S31.32, using signed-magnitude representation.
> +  * Our fixed31_32 is also S31.32, but is using 2's complement. We have
> +  * to convert from signed-magnitude to 2's complement.
> +  */
> + for (i = 0; i < 12; i++) {
> + /* gamut_remap_matrix[i] = ctm[i - floor(i/4)] */
> + matrix[i] = dc_fixpt_from_s3132(ctm->matrix[i]);
> + }
> +}
> +
>  /**
>   * __set_legacy_tf - Calculates the legacy transfer function
>   * @func: transfer function
> @@ -1159,7 +1185,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct 
> dm_crtc_state *crtc,
>  {
>   struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
>   struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
> - struct drm_color_ctm *ctm = NULL;
> + struct drm_color_ctm2 *ctm = NULL;
>   struct dc_color_caps *color_caps = NULL;
>   bool has_crtc_cm_degamma;
>   int ret;
> @@ -1213,7 +1239,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct 
> dm_crtc_state *crtc,
>  
>   /* Setup CRTC CTM. */
>   if (dm_plane_state->ctm) {
> - ctm = (struct drm_color_ctm *)dm_plane_state->ctm->data;
> + ctm = (struct drm_color_ctm2 *)dm_plane_state->ctm->data;
>  
>   /*
>* So far, if we have both plane and CRTC CTM, plane CTM takes
> @@ -1224,7 +1250,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct 
> dm_crtc_state *crtc,
>* provide support for both DPP and MPC matrix at the same
>* time.
>*/
> - __drm_ctm_to_dc_matrix(ctm, 
> dc_plane_state->gamut_remap_matrix.matrix);
> + __drm_ctm2_to_dc_matrix(ctm, 
> dc_plane_state->gamut_remap_matrix.matrix);
>  
>   dc_plane_state->gamut_remap_matrix.enable_remap = true;
>   dc_plane_state->input_csc_color_matrix.enable_adjustment = 
> false;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 0b1081c690cb..27962a3d30f5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1543,7 +1543,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>   ret = drm_property_replace_blob_from_id(plane->dev,
>   _plane_state->ctm,
>   val,
> - sizeof(struct 
> drm_color_ctm), -1,
> + sizeof(struct 
> drm_color_ctm2), -1,

We need to update the comment for dm_plane_state.ctm in amdgpu_dm.h
to specify the property is of type drm_color_ctm2 (or drm_color_ctm_3x4).

>   );
>   dm_plane_state->base.color_mgmt_changed |= replaced;
>   return ret;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 46becedf5b2f..402288133e4c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -838,6 +838,14 @@ struct drm_color_ctm {
>   __u64 matrix[9];
>  };
>  
> +struct drm_color_ctm2 {

Calling this drm_color_ctm_3x4 might be good to make it clear this is
for a 3x4 matrix.

Harry

> + /*
> +  * Conversion matrix in S31.32 sign-magnitude
> +  * (not two's complement!) format.
> +  */
> + __u64 matrix[12];
> +};
> +
>  struct drm_color_lut

Re: [PATCH v2 33/34] drm/amd/display: add plane CTM support




On 2023-08-10 12:03, Melissa Wen wrote:
> Map the plane CTM driver-specific property to DC plane, instead of DC
> stream. The remaining steps to program DPP block are already implemented
> on DC shared-code.
> 
> Signed-off-by: Melissa Wen 
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  1 +
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 25 +++
>  2 files changed, 26 insertions(+)
> 
> 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 dfe61c5ed49e..f239410234b3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -9578,6 +9578,7 @@ static bool should_reset_plane(struct drm_atomic_state 
> *state,
>   if (dm_old_other_state->degamma_tf != 
> dm_new_other_state->degamma_tf ||
>   dm_old_other_state->degamma_lut != 
> dm_new_other_state->degamma_lut ||
>   dm_old_other_state->hdr_mult != 
> dm_new_other_state->hdr_mult ||
> + dm_old_other_state->ctm != dm_new_other_state->ctm ||
>   dm_old_other_state->shaper_lut != 
> dm_new_other_state->shaper_lut ||
>   dm_old_other_state->shaper_tf != 
> dm_new_other_state->shaper_tf ||
>   dm_old_other_state->lut3d != dm_new_other_state->lut3d ||
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 86a918ab82be..7ff329101fd4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -1158,6 +1158,8 @@ int amdgpu_dm_update_plane_color_mgmt(struct 
> dm_crtc_state *crtc,
> struct dc_plane_state *dc_plane_state)
>  {
>   struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
> + struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
> + struct drm_color_ctm *ctm = NULL;
>   struct dc_color_caps *color_caps = NULL;
>   bool has_crtc_cm_degamma;
>   int ret;
> @@ -1209,6 +1211,29 @@ int amdgpu_dm_update_plane_color_mgmt(struct 
> dm_crtc_state *crtc,
>   return ret;
>   }
>  
> + /* Setup CRTC CTM. */
> + if (dm_plane_state->ctm) {
> + ctm = (struct drm_color_ctm *)dm_plane_state->ctm->data;
> +
> + /*
> +  * So far, if we have both plane and CRTC CTM, plane CTM takes
> +  * the priority and we discard data for CRTC CTM, as
> +  * implemented in dcn10_program_gamut_remap().  However, we

Isn't it the opposite? If stream (crtc) has a CTM we program that, only if
stream doesn't have a CTM we program the plane one?

Harry

> +  * have MPC gamut_remap_matrix from DCN3 family, therefore we
> +  * can remap MPC programing of the matrix to MPC block and
> +  * provide support for both DPP and MPC matrix at the same
> +  * time.
> +  */
> + __drm_ctm_to_dc_matrix(ctm, 
> dc_plane_state->gamut_remap_matrix.matrix);
> +
> + dc_plane_state->gamut_remap_matrix.enable_remap = true;
> + dc_plane_state->input_csc_color_matrix.enable_adjustment = 
> false;
> + } else {
> + /* Bypass CTM. */
> + dc_plane_state->gamut_remap_matrix.enable_remap = false;
> + dc_plane_state->input_csc_color_matrix.enable_adjustment = 
> false;
> + }
> +
>   return amdgpu_dm_plane_set_color_properties(plane_state,
>   dc_plane_state, color_caps);
>  }

RE: [PATCH v6 4/4] drm/bridge: panel: Drop CONFIG_OF conditional around *_of_get_bridge()

2023-09-06 Thread Biju Das

Hi Doug Anderson,

> Subject: Re: [PATCH v6 4/4] drm/bridge: panel: Drop CONFIG_OF conditional
> around *_of_get_bridge()
> 
> Hi,
> 
> On Thu, Aug 31, 2023 at 1:10 AM Biju Das 
> wrote:
> >
> > Drop unnecessary CONFIG_OF conditional around devm_drm_of_get_bridge()
> > and
> > drmm_of_get_bridge() as it is guarded with #if..#else blocks in
> > drm_bridge.h.
> >
> > Signed-off-by: Biju Das 
> > ---
> > v6:
> >  * New patch.
> > ---
> >  drivers/gpu/drm/bridge/panel.c | 3 ---
> >  1 file changed, 3 deletions(-)
> 
> Just to be explicit, I'm _not_ landing this patch though I've landed
> patches #1 - #3 from this series.

Thank you.

Cheers,
Biju

[PATCH 0/2] Add st7735s drm driver and Winstar panel

2023-09-06 Thread Stefan x Nilsson

Add a new driver for the Sitronix st7735s display controller
together with a 0.96" 80x160 color TFT display by Winstar.

The driver is very similar to the st7735r driver, but uses a
different pipe_enable sequence and also allows for an
optional regulator to be specified using devicetree.

Signed-off-by: Stefan x Nilsson 
---
Stefan x Nilsson (2):
  dt-bindings: display: Add st7735s driver
  drm: tiny: Add st7735s driver

 .../bindings/display/sitronix,st7735s.yaml |  81 +++
 MAINTAINERS|   7 +
 drivers/gpu/drm/tiny/Kconfig   |  14 ++
 drivers/gpu/drm/tiny/Makefile  |   1 +
 drivers/gpu/drm/tiny/st7735s.c | 264 +
 5 files changed, 367 insertions(+)
---
base-commit: b715dcd3db4a9a57b3fbe7820db37cae930f0867
change-id: 20230825-st7735s-fb9e20e81027

Best regards,
-- 
Stefan x Nilsson

[PATCH 2/2] drm: tiny: Add st7735s driver

2023-09-06 Thread Stefan x Nilsson

Add a driver for Sitronix st7735s display controller, as well as a
Winstar wf0096atyaa3dnn0 0.96" 80x160 TFT panel.

The driver code is very similar to st7735r, but with adaptations for
the pipe_enable function. There is also optional support to specify
a power regulator for the display.

Signed-off-by: Stefan x Nilsson 
---
 MAINTAINERS|   1 +
 drivers/gpu/drm/tiny/Kconfig   |  14 +++
 drivers/gpu/drm/tiny/Makefile  |   1 +
 drivers/gpu/drm/tiny/st7735s.c | 264 +
 4 files changed, 280 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c00b2b9086f2..f24295d691e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6733,6 +6733,7 @@ M:Stefan x Nilsson 
 S: Maintained
 T: git git://anongit.freedesktop.org/drm/drm-misc
 F: Documentation/devicetree/bindings/display/sitronix,st7735s.yaml
+F: drivers/gpu/drm/tiny/st7735s.c
 
 DRM DRIVER FOR SOLOMON SSD130X OLED DISPLAYS
 M: Javier Martinez Canillas 
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index f6889f649bc1..2917f5412ddd 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -212,3 +212,17 @@ config TINYDRM_ST7735R
  * Okaya RH128128T 1.44" 128x128 TFT
 
  If M is selected the module will be called st7735r.
+
+config TINYDRM_ST7735S
+   tristate "DRM support for Sitronix ST7735S display panels"
+   depends on DRM && SPI
+   select DRM_KMS_HELPER
+   select DRM_GEM_DMA_HELPER
+   select DRM_MIPI_DBI
+   select BACKLIGHT_CLASS_DEVICE
+   help
+ DRM driver for Sitronix ST7735S with one of the following
+ LCDs:
+ * Winstar WF0096ATYAA3DNN0 0.96" 80x160 Color TFT
+
+ If M is selected the module will be called st7735s.
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 76dde89a044b..2e805c5b6f16 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_TINYDRM_MI0283QT)+= mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)  += repaper.o
 obj-$(CONFIG_TINYDRM_ST7586)   += st7586.o
 obj-$(CONFIG_TINYDRM_ST7735R)  += st7735r.o
+obj-$(CONFIG_TINYDRM_ST7735S)  += st7735s.o
diff --git a/drivers/gpu/drm/tiny/st7735s.c b/drivers/gpu/drm/tiny/st7735s.c
new file mode 100644
index ..42290f4128db
--- /dev/null
+++ b/drivers/gpu/drm/tiny/st7735s.c
@@ -0,0 +1,264 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DRM driver for display panels connected to a Sitronix ST7735S
+ * display controller in SPI mode.
+ *
+ * Copyright (C) 2023 Axis Communications AB
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ST7735S_FRMCTR10xb1
+#define ST7735S_FRMCTR20xb2
+#define ST7735S_FRMCTR30xb3
+#define ST7735S_INVCTR 0xb4
+#define ST7735S_PWCTR1 0xc0
+#define ST7735S_PWCTR2 0xc1
+#define ST7735S_PWCTR3 0xc2
+#define ST7735S_PWCTR4 0xc3
+#define ST7735S_PWCTR5 0xc4
+#define ST7735S_VMCTR1 0xc5
+#define ST7735S_GAMCTRP1   0xe0
+#define ST7735S_GAMCTRN1   0xe1
+
+#define ST7735S_MY BIT(7)
+#define ST7735S_MX BIT(6)
+#define ST7735S_MV BIT(5)
+#define ST7735S_RGBBIT(3)
+
+struct st7735s_cfg {
+   const struct drm_display_mode mode;
+   unsigned int left_offset;
+   unsigned int top_offset;
+   unsigned int write_only:1;
+   unsigned int rgb:1; /* RGB (vs. BGR) */
+};
+
+struct st7735s_priv {
+   struct mipi_dbi_dev dbidev; /* Must be first for .release() */
+   const struct st7735s_cfg *cfg;
+};
+
+static void st7735s_pipe_enable(struct drm_simple_display_pipe *pipe,
+   struct drm_crtc_state *crtc_state,
+   struct drm_plane_state *plane_state)
+{
+   struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+   struct st7735s_priv *priv = container_of(dbidev, struct st7735s_priv,
+dbidev);
+   struct mipi_dbi *dbi = >dbi;
+   int ret, idx;
+   u8 addr_mode;
+
+   if (!drm_dev_enter(pipe->crtc.dev, ))
+   return;
+
+   DRM_DEBUG_KMS("\n");
+
+   ret = mipi_dbi_poweron_reset(dbidev);
+   if (ret)
+   goto out_exit;
+
+   mipi_dbi_command(dbi, MIPI_DCS_EXIT_SLEEP_MODE);
+   msleep(120);
+   mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
+
+   mipi_dbi_command(dbi, ST7735S_FRMCTR1, 0x05, 0x3c, 0x3c);
+   mipi_dbi_command(dbi, ST7735S_FRMCTR2, 0x05, 0x3c, 0x3c);
+   mipi_dbi_command(dbi, ST7735S_FRMCTR3, 0x05, 0x3c, 0x3c, 0x05, 0x3c, 
0x3c);
+   mipi_dbi_command(dbi, ST7735S_INVCTR, 0x07);
+   mipi_dbi_command(dbi, ST7735S_PWCTR1, 0xe9,

[PATCH 1/2] dt-bindings: display: Add st7735s driver

2023-09-06 Thread Stefan x Nilsson

Add bindings for a driver for Sitronix st7735s display controller, as
well as for a Winstar wf0096atyaa3dnn0 0.96" 80x160 TFT panel.

Signed-off-by: Stefan x Nilsson 
---
 .../bindings/display/sitronix,st7735s.yaml | 81 ++
 MAINTAINERS|  6 ++
 2 files changed, 87 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/sitronix,st7735s.yaml 
b/Documentation/devicetree/bindings/display/sitronix,st7735s.yaml
new file mode 100644
index ..36234ec22fe2
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/sitronix,st7735s.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/sitronix,st7735s.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sitronix ST7735S Display Panels
+
+maintainers:
+  - Stefan x Nilsson 
+
+description:
+  This binding is for display panels using a Sitronix ST7735S
+  controller in SPI mode.
+
+allOf:
+  - $ref: panel/panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+oneOf:
+  - description:
+  Winstar WF0096ATYAA3DNN0 0.96" 80x160 Color TFT
+items:
+  - enum:
+  - winstar,wf0096atyaa3dnn0
+  - const: sitronix,st7735s
+
+  dc-gpios:
+maxItems: 1
+description: Display data/command selection (D/CX)
+
+  backlight: true
+  reg: true
+  spi-max-frequency: true
+  reset-gpios: true
+  rotation: true
+
+required:
+  - compatible
+  - reg
+  - dc-gpios
+
+additionalProperties: true
+
+examples:
+  - |
+#include 
+
+backlight: backlight {
+compatible = "gpio-backlight";
+gpios = < 44 GPIO_ACTIVE_HIGH>;
+};
+
+regdisplay: regulatordisplay {
+compatible = "regulator-fixed";
+regulator-name = "display";
+regulator-min-microvolt = <330>;
+regulator-max-microvolt = <330>;
+regulator-enable-ramp-delay = <10>;
+enable-active-high;
+};
+
+spi {
+#address-cells = <1>;
+#size-cells = <0>;
+
+display@0 {
+compatible = "winstar,wf0096atyaa3dnn0","sitronix,st7735s";
+reg = <0>;
+spi-max-frequency = <1200>;
+dc-gpios = < 43 GPIO_ACTIVE_HIGH>;
+reset-gpios = < 23 GPIO_ACTIVE_HIGH>;
+backlight = <>;
+power-supply = <>;
+rotation = <270>;
+};
+};
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 6308efa121e1..c00b2b9086f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6728,6 +6728,12 @@ T:   git git://anongit.freedesktop.org/drm/drm-misc
 F: Documentation/devicetree/bindings/display/sitronix,st7735r.yaml
 F: drivers/gpu/drm/tiny/st7735r.c
 
+DRM DRIVER FOR SITRONIX ST7735S PANELS
+M: Stefan x Nilsson 
+S: Maintained
+T: git git://anongit.freedesktop.org/drm/drm-misc
+F: Documentation/devicetree/bindings/display/sitronix,st7735s.yaml
+
 DRM DRIVER FOR SOLOMON SSD130X OLED DISPLAYS
 M: Javier Martinez Canillas 
 S: Maintained

-- 
2.30.2

[PATCH] drm: Rename drm_ioctl_flags() to eliminate duplicate declaration warning

2023-09-06 Thread Juntong Deng

There are 'enum drm_ioctl_flags' and 'bool drm_ioctl_flags(...)' with the
same name, which is not a problem in C, but it can lead to
'WARNING: Duplicate C declaration' when generating documentation.

According to the purpose of the function, rename 'drm_ioctl_flags(...)' to
'drm_check_ioctl_flags(...)' to eliminate the warning.

Signed-off-by: Juntong Deng 
---
 drivers/gpu/drm/drm_ioctl.c | 6 +++---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
 include/drm/drm_ioctl.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index f03ffbacfe9b..30699a0a10bc 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -911,7 +911,7 @@ long drm_ioctl(struct file *filp,
 EXPORT_SYMBOL(drm_ioctl);
 
 /**
- * drm_ioctl_flags - Check for core ioctl and return ioctl permission flags
+ * drm_check_ioctl_flags - Check for core ioctl and return ioctl permission 
flags
  * @nr: ioctl number
  * @flags: where to return the ioctl permission flags
  *
@@ -922,7 +922,7 @@ EXPORT_SYMBOL(drm_ioctl);
  * Returns:
  * True if the @nr corresponds to a DRM core ioctl number, false otherwise.
  */
-bool drm_ioctl_flags(unsigned int nr, unsigned int *flags)
+bool drm_check_ioctl_flags(unsigned int nr, unsigned int *flags)
 {
if (nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END)
return false;
@@ -934,4 +934,4 @@ bool drm_ioctl_flags(unsigned int nr, unsigned int *flags)
*flags = drm_ioctls[nr].flags;
return true;
 }
-EXPORT_SYMBOL(drm_ioctl_flags);
+EXPORT_SYMBOL(drm_check_ioctl_flags);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 8b24ecf60e3e..9615104451b3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1287,7 +1287,7 @@ static long vmw_generic_ioctl(struct file *filp, unsigned 
int cmd,
goto out_io_encoding;
 
flags = ioctl->flags;
-   } else if (!drm_ioctl_flags(nr, ))
+   } else if (!drm_check_ioctl_flags(nr, ))
return -EINVAL;
 
return ioctl_func(filp, cmd, arg);
diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
index 6ed61c371f6c..2fc5fc86f711 100644
--- a/include/drm/drm_ioctl.h
+++ b/include/drm/drm_ioctl.h
@@ -175,7 +175,7 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd, 
unsigned long arg);
 /* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
 #define drm_compat_ioctl NULL
 #endif
-bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
+bool drm_check_ioctl_flags(unsigned int nr, unsigned int *flags);
 
 int drm_noop(struct drm_device *dev, void *data,
 struct drm_file *file_priv);
-- 
2.39.2

Re: [PATCH v2 32/34] drm/amd/display: add plane CTM driver-specific property




On 2023-08-10 12:03, Melissa Wen wrote:
> Plane CTM for pre-blending color space conversion. Only enable
> driver-specific plane CTM property on drivers that support both pre- and
> post-blending gamut remap matrix, i.e., DCN3+ family. Otherwise it
> conflits with DRM CRTC CTM property.
> 
> Signed-off-by: Melissa Wen 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  2 ++
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  7 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   |  7 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   | 20 +++
>  4 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index abb871a912d7..84bf501b02f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -363,6 +363,8 @@ struct amdgpu_mode_info {
>* @plane_hdr_mult_property:
>*/
>   struct drm_property *plane_hdr_mult_property;
> +
> + struct drm_property *plane_ctm_property;
>   /**
>* @shaper_lut_property: Plane property to set pre-blending shaper LUT
>* that converts color content before 3D LUT.
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 095f39f04210..6252ee912a63 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -769,6 +769,13 @@ struct dm_plane_state {
>* S31.32 sign-magnitude.
>*/
>   __u64 hdr_mult;
> + /**
> +  * @ctm:
> +  *
> +  * Color transformation matrix. See drm_crtc_enable_color_mgmt(). The
> +  * blob (if not NULL) is a  drm_color_ctm.
> +  */
> + struct drm_property_blob *ctm;
>   /**
>* @shaper_lut: shaper lookup table blob. The blob (if not NULL) is an
>* array of  drm_color_lut.
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 4356846a2bce..86a918ab82be 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -218,6 +218,13 @@ amdgpu_dm_create_color_properties(struct amdgpu_device 
> *adev)
>   return -ENOMEM;
>   adev->mode_info.plane_hdr_mult_property = prop;
>  
> + prop = drm_property_create(adev_to_drm(adev),
> +DRM_MODE_PROP_BLOB,
> +"AMD_PLANE_CTM", 0);

We'll want to wrap the property creation/attachment with
#ifdef AMD_PRIVATE_COLOR here as well.

Harry

> + if (!prop)
> + return -ENOMEM;
> + adev->mode_info.plane_ctm_property = prop;
> +
>   prop = drm_property_create(adev_to_drm(adev),
>  DRM_MODE_PROP_BLOB,
>  "AMD_PLANE_SHAPER_LUT", 0);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 3fd57de7c5be..0b1081c690cb 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1355,6 +1355,8 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
>  
>   if (dm_plane_state->degamma_lut)
>   drm_property_blob_get(dm_plane_state->degamma_lut);
> + if (dm_plane_state->ctm)
> + drm_property_blob_get(dm_plane_state->ctm);
>   if (dm_plane_state->shaper_lut)
>   drm_property_blob_get(dm_plane_state->shaper_lut);
>   if (dm_plane_state->lut3d)
> @@ -1436,6 +1438,8 @@ static void dm_drm_plane_destroy_state(struct drm_plane 
> *plane,
>  
>   if (dm_plane_state->degamma_lut)
>   drm_property_blob_put(dm_plane_state->degamma_lut);
> + if (dm_plane_state->ctm)
> + drm_property_blob_put(dm_plane_state->ctm);
>   if (dm_plane_state->lut3d)
>   drm_property_blob_put(dm_plane_state->lut3d);
>   if (dm_plane_state->shaper_lut)
> @@ -1473,6 +1477,11 @@ dm_atomic_plane_attach_color_mgmt_properties(struct 
> amdgpu_display_manager *dm,
>  dm->adev->mode_info.plane_hdr_mult_property,
>  AMDGPU_HDR_MULT_DEFAULT);
>  
> + /* Only enable plane CTM if both DPP and MPC gamut remap is available. 
> */
> + if (dm->dc->caps.color.mpc.gamut_remap)
> + drm_object_attach_property(>base,
> +
> dm->adev->mode_info.plane_ctm_property, 0);
> +
>   if (dpp_color_caps.hw_3d_lut) {
>   drm_object_attach_property(>base,
>  mode_info.plane_shaper_lut_property, 
> 0);
> @@ -1530,6 +1539,14 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>   dm_plane_state->hdr_mult = val;
>

[PATCH 2/2] drm/msm/dp: Remove error message when downstream port not connected

Plugging in an Apple dongle without the HDMI cable attached prints out
an error message in the kernel logs when nothing is actually wrong.

   no downstream ports connected

This is because the downstream port for the HDMI connector is not
connected, so the Apple dongle reports that as a zero sink count device.

Cc: Vinod Polimera 
Cc: Kuogee Hsieh 
Signed-off-by: Stephen Boyd 
---
 drivers/gpu/drm/msm/dp/dp_panel.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c 
b/drivers/gpu/drm/msm/dp/dp_panel.c
index 97ba41593820..ae778e1a6fd0 100644
--- a/drivers/gpu/drm/msm/dp/dp_panel.c
+++ b/drivers/gpu/drm/msm/dp/dp_panel.c
@@ -156,7 +156,6 @@ int dp_panel_read_sink_caps(struct dp_panel *dp_panel,
if (drm_dp_is_branch(dp_panel->dpcd)) {
count = drm_dp_read_sink_count(panel->aux);
if (!count) {
-   DRM_ERROR("no downstream ports connected\n");
panel->link->sink_count = 0;
return -ENOTCONN;
}
-- 
https://chromeos.dev

[PATCH 1/2] drm/msm/dp: Inline dp_display_is_sink_count_zero()

This function is basically a one-liner when you ignore the debug
logging. Just inline the function and drop the log to simplify the code.

Suggested-by: Dmitry Baryshkov 
Cc: Vinod Polimera 
Cc: Kuogee Hsieh 
Signed-off-by: Stephen Boyd 
---
 drivers/gpu/drm/msm/dp/dp_display.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
b/drivers/gpu/drm/msm/dp/dp_display.c
index 96bbf6fec2f1..2a5f1ab9a65b 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -341,14 +341,6 @@ static const struct component_ops dp_display_comp_ops = {
.unbind = dp_display_unbind,
 };
 
-static bool dp_display_is_sink_count_zero(struct dp_display_private *dp)
-{
-   drm_dbg_dp(dp->drm_dev, "present=%#x sink_count=%d\n",
-   dp->panel->dpcd[DP_DOWNSTREAMPORT_PRESENT],
-   dp->link->sink_count);
-   return drm_dp_is_branch(dp->panel->dpcd) && dp->link->sink_count == 0;
-}
-
 static void dp_display_send_hpd_event(struct msm_dp *dp_display)
 {
struct dp_display_private *dp;
@@ -507,7 +499,7 @@ static int dp_display_handle_port_ststus_changed(struct 
dp_display_private *dp)
 {
int rc = 0;
 
-   if (dp_display_is_sink_count_zero(dp)) {
+   if (drm_dp_is_branch(dp->panel->dpcd) && dp->link->sink_count == 0) {
drm_dbg_dp(dp->drm_dev, "sink count is zero, nothing to do\n");
if (dp->hpd_state != ST_DISCONNECTED) {
dp->hpd_state = ST_DISCONNECT_PENDING;
-- 
https://chromeos.dev

[PATCH 0/2] drm/msm/dp: More DPCD cleanups

This is a follow-on to this series[1]. I can resend the whole pile if
desired.

[1] https://lore.kernel.org/r/20230829184735.2841739-1-swb...@chromium.org

Stephen Boyd (2):
  drm/msm/dp: Inline dp_display_is_sink_count_zero()
  drm/msm/dp: Remove error message when downstream port not connected

 drivers/gpu/drm/msm/dp/dp_display.c | 10 +-
 drivers/gpu/drm/msm/dp/dp_panel.c   |  1 -
 2 files changed, 1 insertion(+), 10 deletions(-)


base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
prerequisite-patch-id: c637903336fb0fd5593f0016f0c863305c43a6b9
prerequisite-patch-id: 8610693078de9cd5041266c70c4d044d15a5f701
prerequisite-patch-id: e10675f41cc68dcefa566f7f288b2df72afdb116
prerequisite-patch-id: 63280d764b830e3d25455ae642840cff5f90e118
prerequisite-patch-id: 567e00d48c5a296b454079a51483f2acce357347
prerequisite-patch-id: 1c18472728edc1ca8800dd6ed6ff12cb98084ea8
prerequisite-patch-id: c6f74b922b3a4f2255bcdab11fe3a2ecf7891262
-- 
https://chromeos.dev

Re: [PATCH v2 31/34] drm/amd/display: set stream gamut remap matrix to MPC for DCN301




On 2023-08-28 04:20, Pekka Paalanen wrote:
> On Fri, 25 Aug 2023 13:37:08 -0100
> Melissa Wen  wrote:
> 
>> On 08/22, Pekka Paalanen wrote:
>>> On Thu, 10 Aug 2023 15:03:11 -0100
>>> Melissa Wen  wrote:
>>>   
 dc->caps.color.mpc.gamut_remap says there is a post-blending color block
 for gamut remap matrix for DCN3 HW family and newer versions. However,
 those drivers still follow DCN10 programming that remap stream
 gamut_remap_matrix to DPP (pre-blending).  
>>>
>>> That's ok only as long as CRTC degamma is pass-through. Blending itself
>>> is a linear operation, so it doesn't matter if a matrix is applied to
>>> the blending result or to all blending inputs. But you cannot move a
>>> matrix operation to the other side of a non-linear operation, and you
>>> cannot move a non-linear operation across blending.  
>>
>> Oh, I'm not moving it, what I'm doing here is the opposite and fixing
>> it. This patch puts each pre- and post-blending CTM in their right
>> place, since we have the HW caps for it on DCN3+... Or are you just
>> pointing out the implementation mistake on old driver versions?
> 
> It's just the old mistake.
> 
> I hope no-one complains, forcing you to revert this fix as a regression.
> 

I'm worried this will break other OSes since its in DC and shared. I'll
check with Kruno when he's back from vacation. But most likely this will
be problematic.

Worst case we can add a new "program_gamut_remap_actually_post_blending"
(with a better name) function to HWSS, expose it in DC, and make sure
amdgpu_dm never calls the old "program_gamut_remap".

I hope nobody relies on the current (IMO broken) behavior on Linux.

Harry

> 
> Thanks,
> pq
> 
> 
 To enable pre-blending and post-blending gamut_remap matrix supports at
 the same time, set stream gamut_remap to MPC and plane gamut_remap to
 DPP for DCN301 that support both.

 It was tested using IGT KMS color tests for DRM CRTC CTM property and it
 preserves test results.

 Signed-off-by: Melissa Wen 
 ---
  .../drm/amd/display/dc/dcn30/dcn30_hwseq.c| 37 +++
  .../drm/amd/display/dc/dcn30/dcn30_hwseq.h|  3 ++
  .../drm/amd/display/dc/dcn301/dcn301_init.c   |  2 +-
  3 files changed, 41 insertions(+), 1 deletion(-)

Re: [PATCH v2] Remove the parameter not described warning

2023-09-06 Thread Helen Koike


Hi Vinayak,

Thanks for you patch

On 06/09/2023 13:51, Vinayak Hegde wrote:

I encountered a warning during kernel documentation compilation


Usually we write the commit message in imperative mood, please check: 
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes



due to a missing colon in the documentation for the
'num_fences' variable in the sync_file.h header file.
This change adds the missing colon to align with the documentation format.

Signed-off-by: Vinayak Hegde 
---
  include/uapi/linux/sync_file.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/sync_file.h b/include/uapi/linux/sync_file.h
index 7e42a5b7558b..b389a5495181 100644
--- a/include/uapi/linux/sync_file.h
+++ b/include/uapi/linux/sync_file.h


Since you are modifying this file, if you run:
git log --oneline include/uapi/linux/sync_file.h
you will notice that almost all changes start with "dma-buf/sync_file:" 
prefix, it would be nice to maintain the style pattern for the commit title.


Regards,
Helen


@@ -56,7 +56,7 @@ struct sync_fence_info {
   * @name: name of fence
   * @status:   status of fence. 1: signaled 0:active <0:error
   * @flags:sync_file_info flags
- * @num_fences number of fences in the sync_file
+ * @num_fences: number of fences in the sync_file
   * @pad:  padding for 64-bit alignment, should always be zero
   * @sync_fence_info: pointer to array of struct _fence_info with all
   * fences in the sync_file

Re: [PATCH v2 3/6] drm_dbg: add trailing newlines to msgs

2023-09-06 Thread jim . cromie

On Wed, Sep 6, 2023 at 10:42 AM Rodrigo Vivi  wrote:
>
> On Mon, Sep 04, 2023 at 08:32:40AM +0200, Andi Shyti wrote:
> > Hi Jim,
> >
> > On Sun, Sep 03, 2023 at 12:46:00PM -0600, Jim Cromie wrote:
> > > By at least strong convention, a print-buffer's trailing newline says
> > > "message complete, send it".  The exception (no TNL, followed by a call
> > > to pr_cont) proves the general rule.
> > >
> > > Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
> > > 1288 drm_dbg.  Clean up the remainders, in maintainer sized chunks.
> > >
> > > No functional changes.
> > >
> > > Signed-off-by: Jim Cromie 
> >
> > Reviewed-by: Andi Shyti 
>
> I pushed this i915 one to our drm-intel-next.
> While doing it I have changed the subject to make it clear
> this is 'drm/i915:'.
>
> I believe you should do similar change to all the other patches
> to make it clear in the subject about which domain that commit
> is touching... instead of only 'drm_dbg'.
>

I will do that, and drop the one you've already pushed.
Thank you both.


> i.e.: 183670347b06 ("drm/i915: add trailing newlines to msgs")
> https://cgit.freedesktop.org/drm-intel/commit/?h=drm-intel-next=183670347b060521920a81f84ff7f10e227ebe05
>
> Thanks for the patch,
> Rodrigo.
>
> >
> > Andi

Re: [PATCH 4/4] drm/doc/rfc: Mark GPU VA as complete.

2023-09-06 Thread Rodrigo Vivi

On Mon, Sep 04, 2023 at 11:39:55PM +0200, Danilo Krummrich wrote:
> On 8/31/23 21:17, Rodrigo Vivi wrote:
> > On Tue, Aug 29, 2023 at 12:30:04PM -0400, Rodrigo Vivi wrote:
> > > Nouveau has landed the GPU VA helpers, support and documentation
> > > already and Xe is already using the upstream GPU VA.
> > 
> > Danilo, although this is more on the Xe side and I wouldn't ask you
> > to review our code entirely, I'd like to get your ack here as Daniel
> > recommended. Meaning that we are aligned there and not creating any
> > change on top of GPU VA. Xe is currently using GPU VA directly without
> > any customization.
> > 
> > Link: 
> > https://gitlab.freedesktop.org/drm/xe/kernel/-/commit/ea4ae69e66b2940107e74f240ecb9dae87bf1ff1
> > Link: 
> > https://gitlab.freedesktop.org/drm/xe/kernel/-/commits/drm-xe-next?ref_type=heads
> 
> Acked-by: Danilo Krummrich 
> 
> Just one note: If we end up to agree on [1] few more adjustments are needed.

Thanks. I see Thomas is already engaged there and I believe it will be easier
to change that and align when we are merged. Since that appears to not
break any uapi.

> 
> Otherwise, same as the other commit, where is the paragraph going?

to the new
+Xe – Pre-Merge Goals - Completed
+
https://lore.kernel.org/all/20230829163005.54067-2-rodrigo.v...@intel.com/

> 
> - Danilo
> 
> [1] 
> https://lore.kernel.org/dri-devel/202308221050.ktj8ufma-...@intel.com/T/#m7f3b5a7ff70723332adeea32671578cb95c62f7c
> 
> > 
> > > 
> > > Signed-off-by: Rodrigo Vivi 
> > > ---
> > >   Documentation/gpu/rfc/xe.rst | 36 ++--
> > >   1 file changed, 18 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
> > > index a115526c03e0..b67f8e6a1825 100644
> > > --- a/Documentation/gpu/rfc/xe.rst
> > > +++ b/Documentation/gpu/rfc/xe.rst
> > > @@ -88,24 +88,6 @@ depend on any other patch touching drm_scheduler 
> > > itself that was not yet merged
> > >   through drm-misc. This, by itself, already includes the reach of an 
> > > agreement for
> > >   uniform 1 to 1 relationship implementation / usage across drivers.
> > > -GPU VA
> > > ---
> > > -Two main goals of Xe are meeting together here:
> > > -
> > > -1) Have an uAPI that aligns with modern UMD needs.
> > > -
> > > -2) Early upstream engagement.
> > > -
> > > -RedHat engineers working on Nouveau proposed a new DRM feature to handle 
> > > keeping
> > > -track of GPU virtual address mappings. This is still not merged 
> > > upstream, but
> > > -this aligns very well with our goals and with our VM_BIND. The 
> > > engagement with
> > > -upstream and the port of Xe towards GPUVA is already ongoing.
> > > -
> > > -As a key measurable result, Xe needs to be aligned with the GPU VA and 
> > > working in
> > > -our tree. Missing Nouveau patches should *not* block Xe and any needed 
> > > GPUVA
> > > -related patch should be independent and present on dri-devel or acked by
> > > -maintainers to go along with the first Xe pull request towards drm-next.
> > > -
> > >   ASYNC VM_BIND
> > >   -
> > >   Although having a common DRM level IOCTL for VM_BIND is not a 
> > > requirement to get
> > > @@ -230,3 +212,21 @@ Xe merged, it is mandatory to enforce the overall 
> > > locking scheme for all major
> > >   structs and list (so vm and vma). So, a consensus is needed, and 
> > > possibly some
> > >   common helpers. If helpers are needed, they should be also documented 
> > > in this
> > >   document.
> > > +
> > > +GPU VA
> > > +--
> > > +Two main goals of Xe are meeting together here:
> > > +
> > > +1) Have an uAPI that aligns with modern UMD needs.
> > > +
> > > +2) Early upstream engagement.
> > > +
> > > +RedHat engineers working on Nouveau proposed a new DRM feature to handle 
> > > keeping
> > > +track of GPU virtual address mappings. This is still not merged 
> > > upstream, but
> > > +this aligns very well with our goals and with our VM_BIND. The 
> > > engagement with
> > > +upstream and the port of Xe towards GPUVA is already ongoing.
> > > +
> > > +As a key measurable result, Xe needs to be aligned with the GPU VA and 
> > > working in
> > > +our tree. Missing Nouveau patches should *not* block Xe and any needed 
> > > GPUVA
> > > +related patch should be independent and present on dri-devel or acked by
> > > +maintainers to go along with the first Xe pull request towards drm-next.
> > > -- 
> > > 2.41.0
> > > 
> > 
>

Re: [PATCH v2 29/34] drm/amd/display: allow newer DC hardware to use degamma ROM for PQ/HLG




On 2023-08-10 12:03, Melissa Wen wrote:
> From: Joshua Ashton 
> 
> Need to funnel the color caps through to these functions so it can check
> that the hardware is capable.
> 
> v2:
> - remove redundant color caps assignment on plane degamma map (Harry)
> - pass color caps to degamma params
> 
> Signed-off-by: Joshua Ashton 
> Signed-off-by: Melissa Wen 
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 35 ---
>  1 file changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index f638e5b3a70b..4356846a2bce 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -538,6 +538,7 @@ static int amdgpu_dm_set_atomic_regamma(struct 
> dc_stream_state *stream,
>  /**
>   * __set_input_tf - calculates the input transfer function based on expected
>   * input space.
> + * @caps: dc color capabilities
>   * @func: transfer function
>   * @lut: lookup table that defines the color space
>   * @lut_size: size of respective lut.
> @@ -545,7 +546,7 @@ static int amdgpu_dm_set_atomic_regamma(struct 
> dc_stream_state *stream,
>   * Returns:
>   * 0 in case of success. -ENOMEM if fails.
>   */
> -static int __set_input_tf(struct dc_transfer_func *func,
> +static int __set_input_tf(struct dc_color_caps *caps, struct 
> dc_transfer_func *func,
> const struct drm_color_lut *lut, uint32_t lut_size)
>  {
>   struct dc_gamma *gamma = NULL;
> @@ -562,7 +563,7 @@ static int __set_input_tf(struct dc_transfer_func *func,
>   __drm_lut_to_dc_gamma(lut, gamma, false);
>   }
>  
> - res = mod_color_calculate_degamma_params(NULL, func, gamma, gamma != 
> NULL);
> + res = mod_color_calculate_degamma_params(caps, func, gamma, gamma != 
> NULL);
>  
>   if (gamma)
>   dc_gamma_release();
> @@ -725,7 +726,7 @@ static int amdgpu_dm_atomic_blend_lut(const struct 
> drm_color_lut *blend_lut,
>   func_blend->tf = tf;
>   func_blend->sdr_ref_white_level = SDR_WHITE_LEVEL_INIT_VALUE;
>  
> - ret = __set_input_tf(func_blend, blend_lut, blend_size);
> + ret = __set_input_tf(NULL, func_blend, blend_lut, blend_size);
>   } else {
>   func_blend->type = TF_TYPE_BYPASS;
>   func_blend->tf = TRANSFER_FUNCTION_LINEAR;
> @@ -950,7 +951,8 @@ int amdgpu_dm_update_crtc_color_mgmt(struct dm_crtc_state 
> *crtc)
>  
>  static int
>  map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
> -  struct dc_plane_state *dc_plane_state)
> +  struct dc_plane_state *dc_plane_state,
> +  struct dc_color_caps *caps)
>  {
>   const struct drm_color_lut *degamma_lut;
>   enum dc_transfer_func_predefined tf = TRANSFER_FUNCTION_SRGB;
> @@ -1005,7 +1007,7 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
>   dc_plane_state->in_transfer_func->tf =
>   TRANSFER_FUNCTION_LINEAR;
>  
> - r = __set_input_tf(dc_plane_state->in_transfer_func,
> + r = __set_input_tf(caps, dc_plane_state->in_transfer_func,
>  degamma_lut, degamma_size);
>   if (r)
>   return r;
> @@ -1018,7 +1020,7 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
>   dc_plane_state->in_transfer_func->tf = tf;
>  
>   if (tf != TRANSFER_FUNCTION_SRGB &&
> - !mod_color_calculate_degamma_params(NULL,
> + !mod_color_calculate_degamma_params(caps,
>   
> dc_plane_state->in_transfer_func,
>   NULL, false))
>   return -ENOMEM;
> @@ -1029,7 +1031,8 @@ map_crtc_degamma_to_dc_plane(struct dm_crtc_state *crtc,
>  
>  static int
>  __set_dm_plane_degamma(struct drm_plane_state *plane_state,
> -struct dc_plane_state *dc_plane_state)
> +struct dc_plane_state *dc_plane_state,
> +struct dc_color_caps *color_caps)
>  {
>   struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
>   const struct drm_color_lut *degamma_lut;
> @@ -1060,7 +1063,7 @@ __set_dm_plane_degamma(struct drm_plane_state 
> *plane_state,
>   dc_plane_state->in_transfer_func->type =
>   TF_TYPE_DISTRIBUTED_POINTS;
>  
> - ret = __set_input_tf(dc_plane_state->in_transfer_func,
> + ret = __set_input_tf(color_caps, 
> dc_plane_state->in_transfer_func,
>degamma_lut, degamma_size);
>   if (ret)
>   return ret;
> @@ -1068,7 +1071,7 @@ __set_dm_plane_degamma(struct drm_plane_state 
>

Re: [PATCH 3/4] drm/doc/rfc: Mark DRM_VM_BIND as complete.

2023-09-06 Thread Rodrigo Vivi

On Mon, Sep 04, 2023 at 11:32:30PM +0200, Danilo Krummrich wrote:
> Hi Rodrigo,
> 
> On 8/31/23 21:10, Rodrigo Vivi wrote:
> > On Tue, Aug 29, 2023 at 12:30:03PM -0400, Rodrigo Vivi wrote:
> > > The consensus is for individual drivers VM_BIND uapis with
> > > the GPUVA helpers that are already implemented and merged
> > > upstream.
> > > 
> > > The merged GPUVA documentation also establish some overall
> > > rules for the locking to be followed by the drivers.
> > 
> > Danilo, do you agree with this?
> > if nothing is missing on that front, could you please ack this patch?
> 
> I agree with the above, hence:
> 
> Acked-by: Danilo Krummrich 
> 
> However, the documentation below seems to be more about a common DRM_VM_BIND 
> IOCTL?
> I guess your commit refers to the end where it talks about common VM_BIND 
> helpers.

Yes, it was about a common vm_bind helpers. But if the consensus is to go with
the individual drivers vm_bind uapis with common gpu_va, this common drm_vm_bind
talk makes no sense anymore. So we can end the talks about it.

> 
> Otherwise the patch is moving the "DRM_VM_BIND" paragraph somewhere below the
> "Dev_coredump"paragraph. Is there some kind of "Done-Section" I'm missing?

Yes, it moves to a new
+Xe – Pre-Merge Goals - Completed
+

added on patch 2 with devcoredump:

https://lore.kernel.org/all/20230829163005.54067-2-rodrigo.v...@intel.com/


> 
> - Danilo
> 
> > 
> > > 
> > > Signed-off-by: Rodrigo Vivi 
> > > ---
> > >   Documentation/gpu/rfc/xe.rst | 34 +-
> > >   1 file changed, 17 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/Documentation/gpu/rfc/xe.rst b/Documentation/gpu/rfc/xe.rst
> > > index bf60c5c82d0e..a115526c03e0 100644
> > > --- a/Documentation/gpu/rfc/xe.rst
> > > +++ b/Documentation/gpu/rfc/xe.rst
> > > @@ -106,23 +106,6 @@ our tree. Missing Nouveau patches should *not* block 
> > > Xe and any needed GPUVA
> > >   related patch should be independent and present on dri-devel or acked by
> > >   maintainers to go along with the first Xe pull request towards drm-next.
> > > -DRM_VM_BIND
> > > 
> > > -Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in 
> > > order to
> > > -fulfill the needs of the modern uAPI. Xe merge should *not* be blocked 
> > > on the
> > > -development of a common new drm_infrastructure. However, the Xe team 
> > > needs to
> > > -engage with the community to explore the options of a common API.
> > > -
> > > -As a key measurable result, the DRM_VM_BIND needs to be documented in 
> > > this file
> > > -below, or this entire block deleted if the consensus is for independent 
> > > drivers
> > > -vm_bind ioctls.
> > > -
> > > -Although having a common DRM level IOCTL for VM_BIND is not a 
> > > requirement to get
> > > -Xe merged, it is mandatory to enforce the overall locking scheme for all 
> > > major
> > > -structs and list (so vm and vma). So, a consensus is needed, and 
> > > possibly some
> > > -common helpers. If helpers are needed, they should be also documented in 
> > > this
> > > -document.
> > > -
> > >   ASYNC VM_BIND
> > >   -
> > >   Although having a common DRM level IOCTL for VM_BIND is not a 
> > > requirement to get
> > > @@ -230,3 +213,20 @@ Later, when we are in-tree, the goal is to 
> > > collaborate with devcoredump
> > >   infrastructure with overall possible improvements, like multiple file 
> > > support
> > >   for better organization of the dumps, snapshot support, dmesg extra 
> > > print,
> > >   and whatever may make sense and help the overall infrastructure.
> > > +
> > > +DRM_VM_BIND
> > > +---
> > > +Nouveau, and Xe are all implementing ‘VM_BIND’ and new ‘Exec’ uAPIs in 
> > > order to
> > > +fulfill the needs of the modern uAPI. Xe merge should *not* be blocked 
> > > on the
> > > +development of a common new drm_infrastructure. However, the Xe team 
> > > needs to
> > > +engage with the community to explore the options of a common API.
> > > +
> > > +As a key measurable result, the DRM_VM_BIND needs to be documented in 
> > > this file
> > > +below, or this entire block deleted if the consensus is for independent 
> > > drivers
> > > +vm_bind ioctls.
> > > +
> > > +Although having a common DRM level IOCTL for VM_BIND is not a 
> > > requirement to get
> > > +Xe merged, it is mandatory to enforce the overall locking scheme for all 
> > > major
> > > +structs and list (so vm and vma). So, a consensus is needed, and 
> > > possibly some
> > > +common helpers. If helpers are needed, they should be also documented in 
> > > this
> > > +document.
> > > -- 
> > > 2.41.0
> > > 
> > 
>

Re: [PATCH v2] Remove the parameter not described warning

2023-09-06 Thread Sumit Semwal

Hi Vinayak,

On Wed, 6 Sept 2023 at 22:21, Vinayak Hegde  wrote:
>
> I encountered a warning during kernel documentation compilation
> due to a missing colon in the documentation for the
> 'num_fences' variable in the sync_file.h header file.
> This change adds the missing colon to align with the documentation format.
>
> Signed-off-by: Vinayak Hegde 
Thanks for your patch; I'll queue it via drm-misc.
> ---
>  include/uapi/linux/sync_file.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/sync_file.h b/include/uapi/linux/sync_file.h
> index 7e42a5b7558b..b389a5495181 100644
> --- a/include/uapi/linux/sync_file.h
> +++ b/include/uapi/linux/sync_file.h
> @@ -56,7 +56,7 @@ struct sync_fence_info {
>   * @name:  name of fence
>   * @status:status of fence. 1: signaled 0:active <0:error
>   * @flags: sync_file_info flags
> - * @num_fences number of fences in the sync_file
> + * @num_fences: number of fences in the sync_file
>   * @pad:   padding for 64-bit alignment, should always be zero
>   * @sync_fence_info: pointer to array of struct _fence_info with all
>   *  fences in the sync_file
> --
> 2.34.1
>

Best,
Sumit.

-- 
Thanks and regards,

Sumit Semwal (he / him)
Tech Lead - LCG, Vertical Technologies
Linaro.org │ Open source software for ARM SoCs

[PATCH v2] Remove the parameter not described warning

2023-09-06 Thread Vinayak Hegde

I encountered a warning during kernel documentation compilation
due to a missing colon in the documentation for the
'num_fences' variable in the sync_file.h header file.
This change adds the missing colon to align with the documentation format.

Signed-off-by: Vinayak Hegde 
---
 include/uapi/linux/sync_file.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/sync_file.h b/include/uapi/linux/sync_file.h
index 7e42a5b7558b..b389a5495181 100644
--- a/include/uapi/linux/sync_file.h
+++ b/include/uapi/linux/sync_file.h
@@ -56,7 +56,7 @@ struct sync_fence_info {
  * @name:  name of fence
  * @status:status of fence. 1: signaled 0:active <0:error
  * @flags: sync_file_info flags
- * @num_fences number of fences in the sync_file
+ * @num_fences: number of fences in the sync_file
  * @pad:   padding for 64-bit alignment, should always be zero
  * @sync_fence_info: pointer to array of struct _fence_info with all
  *  fences in the sync_file
-- 
2.34.1

Re: [PATCH v2 3/6] drm_dbg: add trailing newlines to msgs

2023-09-06 Thread Rodrigo Vivi

On Mon, Sep 04, 2023 at 08:32:40AM +0200, Andi Shyti wrote:
> Hi Jim,
> 
> On Sun, Sep 03, 2023 at 12:46:00PM -0600, Jim Cromie wrote:
> > By at least strong convention, a print-buffer's trailing newline says
> > "message complete, send it".  The exception (no TNL, followed by a call
> > to pr_cont) proves the general rule.
> > 
> > Most DRM.debug calls already comport with this: 207 DRM_DEV_DEBUG,
> > 1288 drm_dbg.  Clean up the remainders, in maintainer sized chunks.
> > 
> > No functional changes.
> > 
> > Signed-off-by: Jim Cromie 
> 
> Reviewed-by: Andi Shyti  

I pushed this i915 one to our drm-intel-next.
While doing it I have changed the subject to make it clear
this is 'drm/i915:'.

I believe you should do similar change to all the other patches
to make it clear in the subject about which domain that commit
is touching... instead of only 'drm_dbg'.

i.e.: 183670347b06 ("drm/i915: add trailing newlines to msgs")
https://cgit.freedesktop.org/drm-intel/commit/?h=drm-intel-next=183670347b060521920a81f84ff7f10e227ebe05

Thanks for the patch,
Rodrigo.

> 
> Andi

Re: [RFC, drm-misc-next v4 3/9] drm/radeon: Implement .be_primary() callback

2023-09-06 Thread Alex Deucher

On Tue, Sep 5, 2023 at 1:25 PM suijingfeng  wrote:
>
> Hi,
>
>
> On 2023/9/5 13:50, Christian König wrote:
> > Am 04.09.23 um 21:57 schrieb Sui Jingfeng:
> >> From: Sui Jingfeng 
> >>
> >> On a machine with multiple GPUs, a Linux user has no control over
> >> which one
> >> is primary at boot time.
> >
> > Question is why is that useful? Should we give users the ability to
> > control that?
> >
> > I don't see an use case for this.
> >
>
> On a specific machine with multiple GPUs mounted, only the
> primary graphics get POST-ed (initialized) by the firmware.
> Therefore the DRM drivers for the rest video cards have to
> work without the prerequisite setups done by firmware, This
> is called as POST.

I think that should be regarded as a bug in the driver that should be
fixed and this would not help with that case.  If a driver can't
initialize a device without aid from the pre-OS environment, that
should be fixed in the driver.  This solution also doesn't fix which
device is selected as the primary by the pre-OS environment.  That can
only be fixed in the pre-OS environment code.

>
> One of the use cases is to test if a specific DRM driver
> would works properly, under the circumstance of not being
> POST-ed, The ast drm driver is the first one which refused
> to work if not being POST-ed by the firmware.
>
> Before apply this series, I was unable make drm/ast as the
> primary video card easily. The problem is that on a multiple
> video card configuration, the monitor connected with my
> AST2400 card not light up. While confusing, a naive programmer
> may suspect the PRIME is not working.
>
> After applied this series and passing ast.modeset=10 on the
> kernel cmd line, I found that the monitor connected with my
> ast2400 video card still black, It doesn't display and It
> doesn't show image to me.

The problem with adding modeset=10 is that it only helps when you have
one GPU driven by that driver in the system.  If you have multiple
GPUs driven by that driver, which one would that apply to?  E.g., what
if you have 2 AMD GPUs in the system.

>
> While in the process of study drm/ast, I know that drm/ast
> driver has the POST code shipped, See the ast_post_gpu() function.
> Then, I was wondering why this function doesn't works.
>
> After a short-time (hasty) debugging, I found that the ast_post_gpu()
> function didn't get run. Because it have something to do with the
> ast->config_mode. Without thinking too much, I hardcoded the
> ast->config_mode as ast_use_p2a, the key point is to force the
> ast_post_gpu() function to run.
>
>
> ```
>
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -132,6 +132,8 @@ static int ast_device_config_init(struct ast_device
> *ast)
>  }
>  }
>
> +   ast->config_mode = ast_use_p2a;
> +
>  switch (ast->config_mode) {
>  case ast_use_defaults:
>  drm_info(dev, "Using default configuration\n");
>
> ```
>
> Then, the monitor light up, it display the Ubuntu greeter to me. Therefore
> my patch is useful, at least for the Linux drm driver tester and developer.
> It allow programmers to test the specific part of a specific driver without
> changing a line of the source code and without the need of sudo authority.
>
> It improves the efficiency of the testing and patch verification. I know
> the PrimaryGPU option of Xorg conf, but this approach will remember the
> setup have been made, you need modify it with root authority each time
> you want to switch the primary. But on the process of rapid developing
> and/or testing for multiple video drivers, with only one computer hardware
> resource available. What we really want is a one-shot command, as provided
> by this series.  So, this is the first use case.
>
>
> The second use case is that sometime the firmware is not reliable.
> While there are thousands of ARM64, PowerPC and Mips servers machine,
> Most of them don't have a good UEFI firmware support. I haven't test the
> drm/amdgpu and drm/radeon at my ARM64 server yet. Because this ARM64
> server always use the platform(BMC) integrated display controller as primary.
> The UEFI firmware of it does not provide options menu to tune.
> So, for the first time, the discrete card because useless, despite more 
> powerful.
> I will take time to carry on the testing, so I will be able to tell more
> in the future.
>
>
> Even on X86, when select the PEG as primary on the UEFI BIOS menu.
> There is no way to tell the bios which one of my three
> discrete video be the primary. Not to mention some old UEFI
> firmware, which doesn't provide a setting at all.
> While the benefit of my approach is the flexibility.
> Yes the i915, amdgpu and radeon are good quality,
> but there may have programmers want to try nouveau.
>
>
> The third use case is that VGAARB is also not reliable, It will
> select a wrong device as primary. Especially on Arm64, Loongarch
> and mips arch etc. And the X server will use this

Re: [PATCH v6 4/4] drm/bridge: panel: Drop CONFIG_OF conditional around *_of_get_bridge()

Hi,

On Thu, Aug 31, 2023 at 1:10 AM Biju Das  wrote:
>
> Drop unnecessary CONFIG_OF conditional around devm_drm_of_get_bridge() and
> drmm_of_get_bridge() as it is guarded with #if..#else blocks in
> drm_bridge.h.
>
> Signed-off-by: Biju Das 
> ---
> v6:
>  * New patch.
> ---
>  drivers/gpu/drm/bridge/panel.c | 3 ---
>  1 file changed, 3 deletions(-)

Just to be explicit, I'm _not_ landing this patch though I've landed
patches #1 - #3 from this series.


-Doug

Re: [PATCH v6 3/4] drm/bridge: Drop CONFIG_OF conditionals around of_node pointers

Hi,

On Thu, Aug 31, 2023 at 1:10 AM Biju Das  wrote:
>
> Having conditional around the of_node pointers turns out to make driver
> code use ugly #ifdef and #if blocks. So drop the conditionals.
>
> Suggested-by: Douglas Anderson 
> Signed-off-by: Biju Das 
> Reviewed-by: Douglas Anderson 
> ---
> v5->v6:
>  * Added Rb tag from Douglas Anderson.
>  * Dropped conditionals from remaining drm/bridge drivers.
> v5:
>  * Split from patch#2
> ---
>  drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 2 --
>  drivers/gpu/drm/bridge/panel.c | 2 --
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c  | 2 --
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c  | 2 --
>  4 files changed, 8 deletions(-)

I think this has had enough time to marinate, so landed to drm-misc-next:

481fc9e7e11d drm/bridge: Drop CONFIG_OF conditionals around of_node pointers

Re: [PATCH v6 1/4] drm/bridge/analogix/anx78xx: Drop ID table

Hi,

On Thu, Aug 31, 2023 at 1:09 AM Biju Das  wrote:
>
> The driver has an ID table, but it uses the wrong API for retrieving match
> data and that will lead to a crash, if it is instantiated by user space or
> using ID. From this, there is no user for the ID table and let's drop it
> from the driver as it saves some memory.
>
> Signed-off-by: Biju Das 
> Reviewed-by: Douglas Anderson 
> Reviewed-by: Laurent Pinchart 
> Reviewed-by: Andy Shevchenko 
> Reviewed-by: Helen Koike 
> ---
> v5->v6:
>  * No change.
> v4->v5:
>  * Added Rb tag from Andy and Helen.
> v3->v4:
>  * Added Rb tag from Laurent and Douglas Anderson.
> v2->v3:
>  * Updated commit header.
> v1->v2:
>  * Dropped ID table support.
> ---
>  drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 7 ---
>  1 file changed, 7 deletions(-)

I think this has had enough time to marinate, so landed to drm-misc-next:

39e0b96d61b6 drm/bridge/analogix/anx78xx: Drop ID table

Re: [PATCH v6 2/4] drm/bridge: Drop conditionals around of_node pointers

Hi,

On Thu, Aug 31, 2023 at 1:10 AM Biju Das  wrote:
>
> This patch is based on commit c9e358dfc4a8 ("driver-core: remove
> conditionals around devicetree pointers").
>
> Having conditional around the of_node pointer of the drm_bridge
> structure turns out to make driver code use ugly #ifdef blocks. Drop the
> conditionals to simplify drivers. While this slightly increases the size
> of struct drm_bridge on non-OF system, the number of bridges used today
> and foreseen tomorrow on those systems is very low, so this shouldn't be
> an issue.
>
> So drop #if conditionals by adding struct device_node forward declaration.
>
> Suggested-by: Douglas Anderson 
> Signed-off-by: Biju Das 
> Reviewed-by: Douglas Anderson 
> Reviewed-by: Laurent Pinchart 
> ---
> v5->v6:
>  * Updated commit description.
>  * Added Rb tag from Douglas Anderson and Laurent
> v5:
>  * Split from patch#2
>  * Updated commit description
>  * Added struct device_node forward declaration.
> ---
>  include/drm/drm_bridge.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I think this has had enough time to marinate, so landed to drm-misc-next:

d8dfccde2709 drm/bridge: Drop conditionals around of_node pointers

Re: [PATCH v3 1/1] backlight: hid_bl: Add VESA VCP HID backlight driver

2023-09-06 Thread Hans de Goede

Hi Julius,

On 9/4/23 21:02, Julius Zint wrote:
> 
> 
> On Mon, 4 Sep 2023, Thomas Weißschuh wrote:
> 
>> +Cc Hans who ins involved with the backlight subsystem
>>
>> Hi Julius,
>>
>> today I stumbled upon a mail from Hans [0], which explains that the
>> backlight subsystem is not actually a good fit (yet?) for external
>> displays.
>>
>> It seems a new API is in the works that would better fit, but I'm not
>> sure about the state of this API. Maybe Hans can clarify.
>>
>> This also ties back to my review question how userspace can figure out
>> to which display a backlight devices applies. So far it can not.
>>
>> [0] 
>> https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfd...@redhat.com/
>>
> 
> Hi Thomas,
> 
> thanks for the hint. I will make sure to give this a proper read and
> see, if it fits my use case better then the current backlight subsystem.

Note the actual proposal for the new usespace API for display brightness
control is here:

https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86...@redhat.com/

I have finished / stabilized the backlight code refactor which I landed
in 6.1, which is a prerequisite for the above proposal. But I have not
been able to make time to actually implement the above proposal; and
I don't know when I will be able to make time for this.

> Especially since I wasnt able to properly address your other review
> comments for now. You are right that the name should align better with
> the kernel module and also, that it is possible for multiple displays to
> be attached.
> 
> In its current state, this would mean that you could only control the
> backlight for the first HID device (enough for me :-).
> 
> The systemd-backlight@.service uses not only the file name, but also the
> full bus path for storing/restoring backlights. I did not yet get around
> to see how the desktops handle brightness control, but since the
> systemd-backlight@.service already uses the name, its important to stay
> the same over multiple boots.
> 
> I would be able to get a handle on the underlying USB device and use the
> serial to uniquely (and persistently) name the backlight. But it does
> feel hacky doing it this way.

So mutter (gnome-shell compositor library) has a similar issue when saving
monitor layouts and I can tell you beforehand that monitor serial numbers
by themselves are not unique enough. Some models just report 123456789
as serial and if you have a dual-monitor setup with 2 such monitors
and name the backlight class device -vcp-hid or something like that
you will still end up with 2 identical names.

To avoid this when saving monitor layouts mutter saves both the port
to which the monitor is attached (e.g. DP-1 DP-2) and the serialnumber
and on startup / monitor hotplug when it checks to see if it has saved
layout info for the monitor it checks the port+serialnr combination.

So what I think you should do is figure out a way to map which
VCP HID device maps to which drm-connector and then use
the connector-name + serial-nr to generate the backlight device name.

We will need the mapping the a drm-connector object anyway for
the new brightness API proposal from above.

Note this does NOT solve the fact that registering a new backlight
class device for an external monitor on a laptop will hopelessly
confuse userspace, see:

https://lore.kernel.org/lkml/7f2d88de-60c5-e2ff-9b22-acba35cfd...@redhat.com/

Regards,

Hans

Re: [PATCH v2] Documentation/gpu: VM_BIND locking document

2023-09-06 Thread Thomas Hellström


Hi, Boris

On 9/6/23 16:54, Boris Brezillon wrote:

Hi Thomas,

On Wed, 6 Sep 2023 16:08:07 +0200
Thomas Hellström  wrote:


Hi, Boris,

On 9/6/23 15:00, Boris Brezillon wrote:

On Wed, 6 Sep 2023 13:57:03 +0200
Thomas Hellström  wrote:
  

Hi, Boris

On 9/6/23 13:09, Boris Brezillon wrote:

On Wed, 6 Sep 2023 10:32:24 +0200
Thomas Hellström  wrote:

 

+Introducing external (or shared) buffer objects
+===
+
+Since shared buffer objects may be shared by multiple gpu_vm's they
+can't share their reservation object with a single gpu_vm, but
will rather
+have a reservation object of their own. The shared objects bound to a
+gpu_vm using one or many
+gpu_vmas are therefore typically put on a per-gpu_vm list which is
+protected by the gpu_vm lock. One could in theory protect it also
with
+the ``gpu_vm->resv``, but since the list of dma_resvs to take is
typically
+built before the ``gpu_vm->resv`` is locked due to a limitation in
+the current locking helpers, that is typically not done. Also see
+below for userptr gpu_vmas.
+
+At eviction time we now need to invalidate *all* gpu_vmas of a shared
+object, but we can no longer be certain that we hold the gpu_vm's
+dma_resv of all the object's gpu_vmas. We can only be certain that we

I need to think a bit more about locking of extobj and evicted
object tracking
in the case of processing 'drm_gpuva_ops' directly through callbacks
within the
fence signalling critical path as mentioend in [1].

In order to support that, we'd need to protect extobjs with a
separate lock,
and while iterating extobjs to acquire the dma-resv lock drop the
lock within
the loop before we actually acquire the dma-resv lock. Maple tree
supports that
already and this can be fully done within the GPUVA manager; no need
for the
driver to care about that.

So do I understand correctly that this because you want to update the
gpuvm state while operations are progressing asynchronously?

If so, I wonder whether that could really be done? For example to
allocate enough memory for page-tables etc, you need to know the
details of the operations at IOCTL execution time, and to know the
details you need to know the state from the previous operation?

Right, sync and async bind can't run fully concurrently, but you could
"inject" a
sync one between two async ones such that the sync ones executed from
the IOCTL
directly while async execution is stalled meanwhile. This would be
possible because
the actual drm_gpuva_ops would be calculated within the async
execution path rather
than in the IOCTL. But yes, page-table management must be desinged to
support that.

FWIW, the panthor driver is designed this way (note that I'm not
supporting GEM eviction yet, so there might be subtleties I missed).

The problem is that once you've published your VM_BIND out-fence, any
code path required to signal that fence may notallocate memory nor or
grab any locks that allows allocating memory while held including
dma_resv locks, and that means all required page-table memory needs to
be allocated synchronously in the IOCTL,

Yep, that's already what I do, by over-provisioning for the worst case
scenario (page table tree is empty), and returning unused pages after
the operation is done.
  

and all evicted bos need to be
made resident in the IOCTL,

Yep, I'm pinning memory to BOs in that path too.
  

and at least in the xe driver the amount of
memory we need to allocate depends on the vm state, so we can't really
update the vm state asynchronously either.

For Mali, we can calculate the maximum amount of pages we'll need for a
MAP operation, by assuming the page table is empty. Then it's just a
matter of returning unused pages to a fast-alloc pool so we can
speed-up further page table allocations (we're using a kmem_cache here,
since the page table update is done by the CPU and memory is shared on
Arm, but there's no reason you can't have your own cache
implementation).
  

But as long as any async binding work required for signalling the
VM_BIND out-fence is properly annotated with
dma_fence_begin_signalling() and dma_fence_end_signalling() and there
aren't any lockdep splats, things should be good. It would trigger on
both memory allocation and attempts to grab a dma_resv lock.

I have dma_fence_{begin,end}_signalling() annotations in the
::run_job() path, and no lockdep complaint spotted so far.
  
  
 

OK, well one of the main motivations for Xe is to be able to pipeline
interleaving binds and execs if needed, like so:

- Bind vmas for scene 1.
- Submit scene 1.
- Unbind vmas for scene 1.
- Bind vmas for scene 2.
- Submit scene 2.
- Unbind vmas for scene 2.

And being able to *submit* all of the above while the async binding of
vmas for scene (step 1) has not yet completed.
I can't really see how this could be done, while obeying dma-fence
rules, unless state is updated synchronously while submitting?

The idea in this case is to detect when a GPU job dependency is a

Re: [PATCH v2] Documentation/gpu: VM_BIND locking document

2023-09-06 Thread Boris Brezillon

Hi Thomas,

On Wed, 6 Sep 2023 16:08:07 +0200
Thomas Hellström  wrote:

> Hi, Boris,
> 
> On 9/6/23 15:00, Boris Brezillon wrote:
> > On Wed, 6 Sep 2023 13:57:03 +0200
> > Thomas Hellström  wrote:
> >  
> >> Hi, Boris
> >>
> >> On 9/6/23 13:09, Boris Brezillon wrote:  
> >>> On Wed, 6 Sep 2023 10:32:24 +0200
> >>> Thomas Hellström  wrote:
> >>>
> >>> 
>  +Introducing external (or shared) buffer objects
>  +===
>  +
>  +Since shared buffer objects may be shared by multiple gpu_vm's they
>  +can't share their reservation object with a single gpu_vm, but
>  will rather
>  +have a reservation object of their own. The shared objects bound to 
>  a
>  +gpu_vm using one or many
>  +gpu_vmas are therefore typically put on a per-gpu_vm list which is
>  +protected by the gpu_vm lock. One could in theory protect it also
>  with
>  +the ``gpu_vm->resv``, but since the list of dma_resvs to take is
>  typically
>  +built before the ``gpu_vm->resv`` is locked due to a limitation in
>  +the current locking helpers, that is typically not done. Also see
>  +below for userptr gpu_vmas.
>  +
>  +At eviction time we now need to invalidate *all* gpu_vmas of a 
>  shared
>  +object, but we can no longer be certain that we hold the gpu_vm's
>  +dma_resv of all the object's gpu_vmas. We can only be certain that 
>  we  
> >>> I need to think a bit more about locking of extobj and evicted
> >>> object tracking
> >>> in the case of processing 'drm_gpuva_ops' directly through callbacks
> >>> within the
> >>> fence signalling critical path as mentioend in [1].
> >>>
> >>> In order to support that, we'd need to protect extobjs with a
> >>> separate lock,
> >>> and while iterating extobjs to acquire the dma-resv lock drop the
> >>> lock within
> >>> the loop before we actually acquire the dma-resv lock. Maple tree
> >>> supports that
> >>> already and this can be fully done within the GPUVA manager; no need
> >>> for the
> >>> driver to care about that.  
> >> So do I understand correctly that this because you want to update the
> >> gpuvm state while operations are progressing asynchronously?
> >>
> >> If so, I wonder whether that could really be done? For example to
> >> allocate enough memory for page-tables etc, you need to know the
> >> details of the operations at IOCTL execution time, and to know the
> >> details you need to know the state from the previous operation?  
> > Right, sync and async bind can't run fully concurrently, but you could
> > "inject" a
> > sync one between two async ones such that the sync ones executed from
> > the IOCTL
> > directly while async execution is stalled meanwhile. This would be
> > possible because
> > the actual drm_gpuva_ops would be calculated within the async
> > execution path rather
> > than in the IOCTL. But yes, page-table management must be desinged to
> > support that.  
> >>> FWIW, the panthor driver is designed this way (note that I'm not
> >>> supporting GEM eviction yet, so there might be subtleties I missed).  
> >> The problem is that once you've published your VM_BIND out-fence, any
> >> code path required to signal that fence may notallocate memory nor or
> >> grab any locks that allows allocating memory while held including
> >> dma_resv locks, and that means all required page-table memory needs to
> >> be allocated synchronously in the IOCTL,  
> > Yep, that's already what I do, by over-provisioning for the worst case
> > scenario (page table tree is empty), and returning unused pages after
> > the operation is done.
> >  
> >> and all evicted bos need to be
> >> made resident in the IOCTL,  
> > Yep, I'm pinning memory to BOs in that path too.
> >  
> >> and at least in the xe driver the amount of
> >> memory we need to allocate depends on the vm state, so we can't really
> >> update the vm state asynchronously either.  
> > For Mali, we can calculate the maximum amount of pages we'll need for a
> > MAP operation, by assuming the page table is empty. Then it's just a
> > matter of returning unused pages to a fast-alloc pool so we can
> > speed-up further page table allocations (we're using a kmem_cache here,
> > since the page table update is done by the CPU and memory is shared on
> > Arm, but there's no reason you can't have your own cache
> > implementation).
> >  
> >> But as long as any async binding work required for signalling the
> >> VM_BIND out-fence is properly annotated with
> >> dma_fence_begin_signalling() and dma_fence_end_signalling() and there
> >> aren't any lockdep splats, things should be good. It would trigger on
> >> both memory allocation and attempts to grab a dma_resv lock.  
> > I have

Re: [PATCH v2 19/34] drm/amd/display: decouple steps for mapping CRTC degamma to DC plane




On 2023-08-29 04:51, Pekka Paalanen wrote:
> On Mon, 28 Aug 2023 12:56:04 -0100
> Melissa Wen  wrote:
> 
>> On 08/28, Pekka Paalanen wrote:
>>> On Mon, 28 Aug 2023 09:45:44 +0100
>>> Joshua Ashton  wrote:
>>>   
 Degamma has always been on the plane on AMD. CRTC DEGAMMA_LUT has actually
 just been applying it to every plane pre-blend.  
>>>
>>> I've never seen that documented anywhere.
>>>
>>> It has seemed obvious, that since we have KMS objects for planes and
>>> CRTCs, stuff on the CRTC does not do plane stuff before blending. That
>>> also has not been documented in the past, but it seemed the most
>>> logical choice.
>>>
>>> Even today
>>> https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#color-management-properties
>>> make no mention of whether they apply before or after blending.  
>>
>> It's mentioned in the next section:
>> https://dri.freedesktop.org/docs/drm/gpu/amdgpu/display/display-manager.html#dc-color-capabilities-between-dcn-generations
>> In hindsight, maybe it isn't the best place...
> 
> That is driver-specific documentation. As a userspace dev, I'd never
> look at driver-specific documentation, because I'm interested in the
> KMS UAPI which is supposed to be generic, and therefore documented with
> the DRM "core".
> 
> Maybe kernel reviewers also never look at driver-specific docs to find
> attempts at redefining common KMS properties?
> 
> (I still don't know which definition is prevalent.)
> 
>>>   
 Degamma makes no sense after blending anyway.  
>>>
>>> If the goal is to allow blending in optical or other space, you are
>>> correct. However, APIs do not need to make sense to exist, like most of
>>> the options of "Colorspace" connector property.
>>>
>>> I have always thought the CRTC DEGAMMA only exists to allow the CRTC
>>> CTM to work in linear or other space.
>>>
>>> I have at times been puzzled by what the DEGAMMA and CTM are actually
>>> good for.
>>>   
 The entire point is for it to happen before blending to blend in linear
 space. Otherwise DEGAMMA_LUT and REGAMMA_LUT are the exact same thing...  
>>>
>>> The CRTC CTM is between CRTC DEGAMMA and CRTC GAMMA, meaning they are
>>> not interchangeable.
>>>
>>> I have literally believed that DRM KMS UAPI simply does not support
>>> blending in optical space, unless your framebuffers are in optical
>>> which no-one does, until the color management properties are added to

I think Mario Kleiner had a use-case that made use of that and introduced
FP16 format support in amdgpu.

>>> KMS planes. This never even seemed weird, because non-linear blending
>>> is so common.
>>>
>>> So I have been misunderstanding the CRTC DEGAMMA property forever. Am I
>>> the only one? Do all drivers agree today at what point does CRTC
>>> DEGAMMA apply, before blending on all planes or after blending?
>>>   
>>
>> I'd like to know current userspace cases on Linux of this CRTC DEGAMMA
>> LUT.
> 
> I don't know of any, but that doesn't mean anything.
> 
>>> Does anyone know of any doc about that?  
>>
>> From what I retrieved about the introduction of CRTC color props[1], it
>> seems the main concern at that point was getting a linear space for
>> CTM[2] and CRTC degamma property seems to have followed intel
>> requirements, but didn't find anything about the blending space.
> 
> Right. I've always thought CRTC props apply after blending.
> 
>> AFAIU, we have just interpreted that all CRTC color properties for DRM
>> interface are after blending[3]. Can this be seen in another way?
> 
> Joshua did, and he has a logical point.
> 
> I guess if we really want to know, someone would need review all
> drivers exposing these props, and even check if they changed in the
> past.
> 
> FWIW, the usefulness of (RE)GAMMA (not DEGAMMA) LUT is limited by the
> fact that attempting to represent 1/2.2 power function as a uniformly
> distributed LUT is infeasible due to the approximation errors near zero.
> 

IMO, CRTC should be post-blending. Blending is at the plane/crtc boundary
by design, therefore CRTC properties apply post-blending.

Though I can understand why DEGAMMA can be interpreted to be applied
pre-blending. Though, I think that's wrong for the DRM/KMS model and
should be fixed in amdgpu.

Harry

> 
> Thanks,
> pq
> 
>> [1] https://patchwork.freedesktop.org/series/2720/
>> [2] https://codereview.chromium.org/1182063002
>> [3] https://dri.freedesktop.org/docs/drm/_images/dcn3_cm_drm_current.svg
>>
>>>
>>> If drivers do not agree on the behaviour of a KMS property, then that
>>> property is useless for generic userspace.
>>>
>>>
>>> Thanks,
>>> pq
>>>
>>>   
 On Tuesday, 22 August 2023, Pekka Paalanen 
 wrote:  
> On Thu, 10 Aug 2023 15:02:59 -0100
> Melissa Wen  wrote:
>
>> The next patch adds pre-blending degamma to AMD color mgmt pipeline, but
>> pre-blending degamma caps (DPP) is currently in use to provide DRM CRTC
>> atomic degamma or implict degamma on legacy gamma. Detach degamma usage

[PATCH v2 3/5] arch/powerpc: Remove trailing whitespaces

Fix coding style. No functional changes.

Signed-off-by: Thomas Zimmermann 
---
 arch/powerpc/include/asm/machdep.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/machdep.h 
b/arch/powerpc/include/asm/machdep.h
index 4f6e7d7ee388..933465ed4c43 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -10,7 +10,7 @@
 #include 
 
 struct pt_regs;
-struct pci_bus;
+struct pci_bus;
 struct device_node;
 struct iommu_table;
 struct rtc_time;
@@ -78,8 +78,8 @@ struct machdep_calls {
unsigned char   (*nvram_read_val)(int addr);
void(*nvram_write_val)(int addr, unsigned char val);
ssize_t (*nvram_write)(char *buf, size_t count, loff_t *index);
-   ssize_t (*nvram_read)(char *buf, size_t count, loff_t *index);  
-   ssize_t (*nvram_size)(void);
+   ssize_t (*nvram_read)(char *buf, size_t count, loff_t *index);
+   ssize_t (*nvram_size)(void);
void(*nvram_sync)(void);
 
/* Exception handlers */
@@ -102,9 +102,9 @@ struct machdep_calls {
 */
long(*feature_call)(unsigned int feature, ...);
 
-   /* Get legacy PCI/IDE interrupt mapping */ 
+   /* Get legacy PCI/IDE interrupt mapping */
int (*pci_get_legacy_ide_irq)(struct pci_dev *dev, int 
channel);
-   
+
/* Get access protection for /dev/mem */
pgprot_t(*phys_mem_access_prot)(struct file *file,
unsigned long pfn,
-- 
2.42.0

[PATCH v2 5/5] arch/powerpc: Call internal __phys_mem_access_prot() in fbdev code

Call __phys_mem_access_prot() from the fbdev mmap helper
fb_pgprot_device(). Allows to avoid the file argument of
NULL.

Signed-off-by: Thomas Zimmermann 
---
 arch/powerpc/include/asm/fb.h | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/fb.h b/arch/powerpc/include/asm/fb.h
index 3c7486323178..8e6a7fc4ae86 100644
--- a/arch/powerpc/include/asm/fb.h
+++ b/arch/powerpc/include/asm/fb.h
@@ -8,12 +8,7 @@ static inline pgprot_t fb_pgprot_device(pgprot_t prot,
unsigned long vm_start, unsigned long 
vm_end,
unsigned long offset)
 {
-   /*
-* PowerPC's implementation of phys_mem_access_prot() does
-* not use the file argument. Set it to NULL in preparation
-* of later updates to the interface.
-*/
-   return phys_mem_access_prot(NULL, PHYS_PFN(offset), vm_end - vm_start, 
prot);
+   return __phys_mem_access_prot(PHYS_PFN(offset), vm_end - vm_start, 
prot);
 }
 #define fb_pgprot_device fb_pgprot_device
 
-- 
2.42.0

[PATCH v2 2/5] fbdev: Replace fb_pgprotect() with fb_pgprot_device()

Rename the fbdev mmap helper fb_pgprotect() to fb_pgprot_device().
The helper sets VMA page-access flags for framebuffers in device I/O
memory. The new name follows pgprot_device(), which does the same for
arbitrary devices.

Also clean up the helper's parameters and return value. Instead of
the VMA instance, pass the individial parameters separately: existing
page-access flags, the VMAs start and end addresses and the offset
in the underlying device memory rsp file. Return the new page-access
flags. These changes align fb_pgprot_device() closer with pgprot_device.

Signed-off-by: Thomas Zimmermann 
---
 arch/ia64/include/asm/fb.h   | 15 +++
 arch/m68k/include/asm/fb.h   | 19 ++-
 arch/mips/include/asm/fb.h   | 11 +--
 arch/powerpc/include/asm/fb.h| 13 +
 arch/sparc/include/asm/fb.h  | 15 +--
 arch/x86/include/asm/fb.h| 10 ++
 arch/x86/video/fbdev.c   | 15 ---
 drivers/video/fbdev/core/fb_chrdev.c |  3 ++-
 include/asm-generic/fb.h | 12 ++--
 9 files changed, 58 insertions(+), 55 deletions(-)

diff --git a/arch/ia64/include/asm/fb.h b/arch/ia64/include/asm/fb.h
index 1717b26fd423..2fbad4a9fc15 100644
--- a/arch/ia64/include/asm/fb.h
+++ b/arch/ia64/include/asm/fb.h
@@ -8,17 +8,16 @@
 
 #include 
 
-struct file;
-
-static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
-   unsigned long off)
+static inline pgprot_t fb_pgprot_device(pgprot_t prot,
+   unsigned long vm_start, unsigned long 
vm_end,
+   unsigned long offset)
 {
-   if (efi_range_is_wc(vma->vm_start, vma->vm_end - vma->vm_start))
-   vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+   if (efi_range_is_wc(vm_start, vm_end - vm_start))
+   return pgprot_writecombine(prot);
else
-   vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+   return pgprot_noncached(prot);
 }
-#define fb_pgprotect fb_pgprotect
+#define fb_pgprot_device fb_pgprot_device
 
 static inline void fb_memcpy_fromio(void *to, const volatile void __iomem 
*from, size_t n)
 {
diff --git a/arch/m68k/include/asm/fb.h b/arch/m68k/include/asm/fb.h
index 24273fc7ad91..4acdf5b62871 100644
--- a/arch/m68k/include/asm/fb.h
+++ b/arch/m68k/include/asm/fb.h
@@ -5,26 +5,27 @@
 #include 
 #include 
 
-struct file;
-
-static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
-   unsigned long off)
+static inline pgprot_t fb_pgprot_device(pgprot_t prot,
+   unsigned long vm_start, unsigned long 
vm_end,
+   unsigned long offset)
 {
 #ifdef CONFIG_MMU
 #ifdef CONFIG_SUN3
-   pgprot_val(vma->vm_page_prot) |= SUN3_PAGE_NOCACHE;
+   pgprot_val(prot) |= SUN3_PAGE_NOCACHE;
 #else
if (CPU_IS_020_OR_030)
-   pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE030;
+   pgprot_val(prot) |= _PAGE_NOCACHE030;
if (CPU_IS_040_OR_060) {
-   pgprot_val(vma->vm_page_prot) &= _CACHEMASK040;
+   pgprot_val(prot) &= _CACHEMASK040;
/* Use no-cache mode, serialized */
-   pgprot_val(vma->vm_page_prot) |= _PAGE_NOCACHE_S;
+   pgprot_val(prot) |= _PAGE_NOCACHE_S;
}
 #endif /* CONFIG_SUN3 */
 #endif /* CONFIG_MMU */
+
+   return prot;
 }
-#define fb_pgprotect fb_pgprotect
+#define fb_pgprot_device fb_pgprot_device
 
 #include 
 
diff --git a/arch/mips/include/asm/fb.h b/arch/mips/include/asm/fb.h
index 18b7226403ba..98e63d14a71f 100644
--- a/arch/mips/include/asm/fb.h
+++ b/arch/mips/include/asm/fb.h
@@ -3,14 +3,13 @@
 
 #include 
 
-struct file;
-
-static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
-   unsigned long off)
+static inline pgprot_t fb_pgprot_device(pgprot_t prot,
+   unsigned long vm_start, unsigned long 
vm_end,
+   unsigned long offset)
 {
-   vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+   return pgprot_noncached(prot);
 }
-#define fb_pgprotect fb_pgprotect
+#define fb_pgprot_device fb_pgprot_device
 
 /*
  * MIPS doesn't define __raw_ I/O macros, so the helpers
diff --git a/arch/powerpc/include/asm/fb.h b/arch/powerpc/include/asm/fb.h
index 61e3b8806db6..3c7486323178 100644
--- a/arch/powerpc/include/asm/fb.h
+++ b/arch/powerpc/include/asm/fb.h
@@ -2,23 +2,20 @@
 #ifndef _ASM_FB_H_
 #define _ASM_FB_H_
 
-#include 
-
 #include 
 
-static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
-   unsigned long off)
+static inline pgprot_t fb_pgprot_device(pgprot_t prot,
+

[PATCH v2 4/5] arch/powerpc: Remove file parameter from phys_mem_access_prot code

Remove 'file' parameter from struct machdep_calls.phys_mem_access_prot
and its implementation in pci_phys_mem_access_prot(). The file is not
used on PowerPC. By removing it, a later patch can simplify fbdev's
mmap code, which uses phys_mem_access_prot() on PowerPC.

Signed-off-by: Thomas Zimmermann 
---
 arch/powerpc/include/asm/book3s/pgtable.h | 10 --
 arch/powerpc/include/asm/machdep.h|  3 +--
 arch/powerpc/include/asm/nohash/pgtable.h | 10 --
 arch/powerpc/include/asm/pci.h|  4 +---
 arch/powerpc/kernel/pci-common.c  |  3 +--
 arch/powerpc/mm/mem.c |  8 
 6 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/pgtable.h 
b/arch/powerpc/include/asm/book3s/pgtable.h
index d18b748ea3ae..84e36a572641 100644
--- a/arch/powerpc/include/asm/book3s/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/pgtable.h
@@ -20,9 +20,15 @@ extern void set_pte_at(struct mm_struct *mm, unsigned long 
addr, pte_t *ptep,
 extern int ptep_set_access_flags(struct vm_area_struct *vma, unsigned long 
address,
 pte_t *ptep, pte_t entry, int dirty);
 
+extern pgprot_t __phys_mem_access_prot(unsigned long pfn, unsigned long size,
+  pgprot_t vma_prot);
+
 struct file;
-extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
-unsigned long size, pgprot_t vma_prot);
+static inline pgprot_t phys_mem_access_prot(struct file *file, unsigned long 
pfn,
+   unsigned long size, pgprot_t 
vma_prot)
+{
+   return __phys_mem_access_prot(pfn, size, vma_prot);
+}
 #define __HAVE_PHYS_MEM_ACCESS_PROT
 
 void __update_mmu_cache(struct vm_area_struct *vma, unsigned long address, 
pte_t *ptep);
diff --git a/arch/powerpc/include/asm/machdep.h 
b/arch/powerpc/include/asm/machdep.h
index 933465ed4c43..d31a5ec1550d 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -106,8 +106,7 @@ struct machdep_calls {
int (*pci_get_legacy_ide_irq)(struct pci_dev *dev, int 
channel);
 
/* Get access protection for /dev/mem */
-   pgprot_t(*phys_mem_access_prot)(struct file *file,
-   unsigned long pfn,
+   pgprot_t(*phys_mem_access_prot)(unsigned long pfn,
unsigned long size,
pgprot_t vma_prot);
 
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h 
b/arch/powerpc/include/asm/nohash/pgtable.h
index a6cb6f92..90366b0b3ad9 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -246,9 +246,15 @@ extern int ptep_set_access_flags(struct vm_area_struct 
*vma, unsigned long addre
 
 #define pgprot_writecombine pgprot_noncached_wc
 
+extern pgprot_t __phys_mem_access_prot(unsigned long pfn, unsigned long size,
+  pgprot_t vma_prot);
+
 struct file;
-extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
-unsigned long size, pgprot_t vma_prot);
+static inline pgprot_t phys_mem_access_prot(struct file *file, unsigned long 
pfn,
+   unsigned long size, pgprot_t 
vma_prot)
+{
+   return __phys_mem_access_prot(pfn, size, vma_prot);
+}
 #define __HAVE_PHYS_MEM_ACCESS_PROT
 
 #ifdef CONFIG_HUGETLB_PAGE
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 289f1ec85bc5..34ed4d51c546 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -104,9 +104,7 @@ extern void of_scan_pci_bridge(struct pci_dev *dev);
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
 extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
 
-struct file;
-extern pgprot_tpci_phys_mem_access_prot(struct file *file,
-unsigned long pfn,
+extern pgprot_tpci_phys_mem_access_prot(unsigned long pfn,
 unsigned long size,
 pgprot_t prot);
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index e88d7c9feeec..73f12a17e572 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -521,8 +521,7 @@ int pci_iobar_pfn(struct pci_dev *pdev, int bar, struct 
vm_area_struct *vma)
  * PCI device, it tries to find the PCI device first and calls the
  * above routine
  */
-pgprot_t pci_phys_mem_access_prot(struct file *file,
- unsigned long pfn,
+pgprot_t pci_phys_mem_access_prot(unsigned long pfn,
  unsigned long size,
  pgprot_t prot)
 {
diff --git

[PATCH v2 1/5] fbdev: Avoid file argument in fb_pgprotect()

Only PowerPC's fb_pgprotect() needs the file argument, although
the implementation does not use it. Pass NULL to the internal
helper in preparation of further updates. A later patch will remove
the file parameter from fb_pgprotect().

While at it, replace the shift operation with PHYS_PFN().

Suggested-by: Christophe Leroy 
Signed-off-by: Thomas Zimmermann 
---
 arch/powerpc/include/asm/fb.h | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/fb.h b/arch/powerpc/include/asm/fb.h
index 5f1a2e5f7654..61e3b8806db6 100644
--- a/arch/powerpc/include/asm/fb.h
+++ b/arch/powerpc/include/asm/fb.h
@@ -9,7 +9,12 @@
 static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
unsigned long off)
 {
-   vma->vm_page_prot = phys_mem_access_prot(file, off >> PAGE_SHIFT,
+   /*
+* PowerPC's implementation of phys_mem_access_prot() does
+* not use the file argument. Set it to NULL in preparation
+* of later updates to the interface.
+*/
+   vma->vm_page_prot = phys_mem_access_prot(NULL, PHYS_PFN(off),
 vma->vm_end - vma->vm_start,
 vma->vm_page_prot);
 }
-- 
2.42.0

[PATCH v2 0/5] ppc, fbdev: Clean up fbdev mmap helper