[PATCH v8 0/5] phy: phy-fsl-imx8-mipi-dphy: Add i.MX8qxp LVDS PHY mode support
Hi, This is the v8 series to add i.MX8qxp LVDS PHY mode support for the Mixel PHY in the Freescale i.MX8qxp SoC. The Mixel PHY is MIPI DPHY + LVDS PHY combo, which can works in either MIPI DPHY mode or LVDS PHY mode. The PHY mode is controlled by i.MX8qxp SCU firmware. The PHY driver would call a SCU function to configure the mode. The PHY driver is already supporting the Mixel MIPI DPHY in i.MX8mq SoC, where it appears to be a single MIPI DPHY. Patch 1/5 sets PHY mode in the Northwest Logic MIPI DSI host controller bridge driver, since i.MX8qxp SoC embeds this controller IP to support MIPI DSI displays together with the Mixel PHY. Patch 2/5 allows LVDS PHYs to be configured through the generic PHY functions and through a custom structure added to the generic PHY configuration union. Patch 3/5 converts mixel,mipi-dsi-phy plain text dt binding to json-schema. Patch 4/5 adds dt binding support for the Mixel combo PHY in i.MX8qxp SoC. Patch 5/5 adds the i.MX8qxp LVDS PHY mode support in the Mixel PHY driver. Welcome comments, thanks. v7->v8: * Trivial kernel doc style fix for patch 2/5 - add '*'. v6->v7: * Update the year of copyright for patch 2/5. * Better variable explanation for bits_per_lane_and_dclk_cycle in patch 2/5. * Use marco instead of magic number for CCM and CA values for patch 5/5. * Suppress 'checkpatch --strict' warnings for patch 5/5. v5->v6: * Rebase the series upon v5.17-rc1. * Set PHY mode in ->mode_set() instead of ->pre_enable() in the nwl-dsi bridge driver in patch 1/5 due to the rebase. * Drop Guido's R-b tag on patch 1/5 due to the rebase. v4->v5: * Align kernel-doc style of include/linux/phy/phy-lvds.h to include/linux/phy/phy.h for patch 2/5. (Vinod) * Trivial tweaks on patch 2/5. * Drop Robert's R-b tag on patch 2/5. v3->v4: * Add all R-b tags received from v3 on relevant patches and respin. (Robert) v2->v3: * Improve readability of mixel_dphy_set_mode() in the Mixel PHY driver. (Guido) * Improve the 'clock-names' property in the PHY dt binding. v1->v2: * Convert mixel,mipi-dsi-phy plain text dt binding to json-schema. (Guido) * Print invalid PHY mode in dmesg from the Mixel PHY driver. (Guido) * Add Guido's R-b tag on the patch for the nwl-dsi drm bridge driver. Liu Ying (5): drm/bridge: nwl-dsi: Set PHY mode in nwl_dsi_mode_set() phy: Add LVDS configuration options dt-bindings: phy: Convert mixel,mipi-dsi-phy to json-schema dt-bindings: phy: mixel: mipi-dsi-phy: Add Mixel combo PHY support for i.MX8qxp phy: freescale: phy-fsl-imx8-mipi-dphy: Add i.MX8qxp LVDS PHY mode support .../bindings/phy/mixel,mipi-dsi-phy.txt | 29 -- .../bindings/phy/mixel,mipi-dsi-phy.yaml | 107 +++ drivers/gpu/drm/bridge/nwl-dsi.c | 6 + .../phy/freescale/phy-fsl-imx8-mipi-dphy.c| 276 +- include/linux/phy/phy-lvds.h | 32 ++ include/linux/phy/phy.h | 4 + 6 files changed, 414 insertions(+), 40 deletions(-) delete mode 100644 Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt create mode 100644 Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml create mode 100644 include/linux/phy/phy-lvds.h -- 2.25.1
[PATCH v8 1/5] drm/bridge: nwl-dsi: Set PHY mode in nwl_dsi_mode_set()
The Northwest Logic MIPI DSI host controller embedded in i.MX8qxp
works with a Mixel MIPI DPHY + LVDS PHY combo to support either
a MIPI DSI display or a LVDS display. So, this patch calls
phy_set_mode() from nwl_dsi_mode_set() to set PHY mode to MIPI DPHY
explicitly.
Cc: Guido Günther
Cc: Robert Chiras
Cc: Martin Kepplinger
Cc: Andrzej Hajda
Cc: Neil Armstrong
Cc: Laurent Pinchart
Cc: Jonas Karlman
Cc: Jernej Skrabec
Cc: David Airlie
Cc: Daniel Vetter
Cc: NXP Linux Team
Signed-off-by: Liu Ying
---
v7->v8:
* No change.
v6->v7:
* No change.
v5->v6:
* Rebase the series upon v5.17-rc1.
* Set PHY mode in ->mode_set() instead of ->pre_enable() in the nwl-dsi
bridge driver due to the rebase.
* Drop Guido's R-b tag due to the rebase.
v4->v5:
* No change.
v3->v4:
* No change.
v2->v3:
* No change.
v1->v2:
* Add Guido's R-b tag.
drivers/gpu/drm/bridge/nwl-dsi.c | 6 ++
1 file changed, 6 insertions(+)
drivers/gpu/drm/bridge/nwl-dsi.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
index d5945501a5ee..85bab7372af1 100644
--- a/drivers/gpu/drm/bridge/nwl-dsi.c
+++ b/drivers/gpu/drm/bridge/nwl-dsi.c
@@ -666,6 +666,12 @@ static int nwl_dsi_mode_set(struct nwl_dsi *dsi)
return ret;
}
+ ret = phy_set_mode(dsi->phy, PHY_MODE_MIPI_DPHY);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev, "Failed to set DSI phy mode: %d\n", ret);
+ goto uninit_phy;
+ }
+
ret = phy_configure(dsi->phy, phy_cfg);
if (ret < 0) {
DRM_DEV_ERROR(dev, "Failed to configure DSI phy: %d\n", ret);
--
2.25.1
[PATCH v8 2/5] phy: Add LVDS configuration options
This patch allows LVDS PHYs to be configured through
the generic functions and through a custom structure
added to the generic union.
The parameters added here are based on common LVDS PHY
implementation practices. The set of parameters
should cover all potential users.
Cc: Kishon Vijay Abraham I
Cc: Vinod Koul
Cc: NXP Linux Team
Signed-off-by: Liu Ying
---
v7->v8:
* Trivial kernel doc style fix - add '*'.
v6->v7:
* Update the year of copyright.
* Better variable explanation for bits_per_lane_and_dclk_cycle.
v5->v6:
* Rebase upon v5.17-rc1.
v4->v5:
* Align kernel-doc style to include/linux/phy/phy.h. (Vinod)
* Trivial tweaks.
* Drop Robert's R-b tag.
v3->v4:
* Add Robert's R-b tag.
v2->v3:
* No change.
v1->v2:
* No change.
include/linux/phy/phy-lvds.h | 32
include/linux/phy/phy.h | 4
2 files changed, 36 insertions(+)
create mode 100644 include/linux/phy/phy-lvds.h
diff --git a/include/linux/phy/phy-lvds.h b/include/linux/phy/phy-lvds.h
new file mode 100644
index ..09931d080a6d
--- /dev/null
+++ b/include/linux/phy/phy-lvds.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2020,2022 NXP
+ */
+
+#ifndef __PHY_LVDS_H_
+#define __PHY_LVDS_H_
+
+/**
+ * struct phy_configure_opts_lvds - LVDS configuration set
+ * @bits_per_lane_and_dclk_cycle: Number of bits per lane per differential
+ * clock cycle.
+ * @differential_clk_rate: Clock rate, in Hertz, of the LVDS
+ * differential clock.
+ * @lanes: Number of active, consecutive,
+ * data lanes, starting from lane 0,
+ * used for the transmissions.
+ * @is_slave: Boolean, true if the phy is a slave
+ * which works together with a master
+ * phy to support dual link transmission,
+ * otherwise a regular phy or a master phy.
+ *
+ * This structure is used to represent the configuration state of a LVDS phy.
+ */
+struct phy_configure_opts_lvds {
+ unsigned intbits_per_lane_and_dclk_cycle;
+ unsigned long differential_clk_rate;
+ unsigned intlanes;
+ boolis_slave;
+};
+
+#endif /* __PHY_LVDS_H_ */
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index f3286f4cd306..b1413757fcc3 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -17,6 +17,7 @@
#include
#include
+#include
#include
struct phy;
@@ -57,10 +58,13 @@ enum phy_media {
* the MIPI_DPHY phy mode.
* @dp:Configuration set applicable for phys supporting
* the DisplayPort protocol.
+ * @lvds: Configuration set applicable for phys supporting
+ * the LVDS 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;
};
/**
--
2.25.1
[PATCH v8 3/5] dt-bindings: phy: Convert mixel, mipi-dsi-phy to json-schema
This patch converts the mixel,mipi-dsi-phy binding to
DT schema format using json-schema.
Comparing to the plain text version, the new binding adds
the 'assigned-clocks', 'assigned-clock-parents' and
'assigned-clock-rates' properites, otherwise 'make dtbs_check'
would complain that there are mis-matches. Also, the new
binding requires the 'power-domains' property since all potential
SoCs that embed this PHY would provide a power domain for it.
The example of the new binding takes reference to the latest
dphy node in imx8mq.dtsi.
Cc: Guido Günther
Cc: Kishon Vijay Abraham I
Cc: Vinod Koul
Cc: Rob Herring
Cc: NXP Linux Team
Reviewed-by: Rob Herring
Reviewed-by: Guido Günther
Signed-off-by: Liu Ying
---
v7->v8:
* No change.
v6->v7:
* No change.
v5->v6:
* No change.
v4->v5:
* No change.
v3->v4:
* Add Rob's and Guido's R-b tags.
v2->v3:
* Improve the 'clock-names' property by dropping 'items:'.
v1->v2:
* Newly introduced in v2. (Guido)
.../bindings/phy/mixel,mipi-dsi-phy.txt | 29
.../bindings/phy/mixel,mipi-dsi-phy.yaml | 72 +++
2 files changed, 72 insertions(+), 29 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt
create mode 100644
Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml
diff --git a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt
b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt
deleted file mode 100644
index 9b23407233c0..
--- a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-Mixel DSI PHY for i.MX8
-
-The Mixel MIPI-DSI PHY IP block is e.g. found on i.MX8 platforms (along the
-MIPI-DSI IP from Northwest Logic). It represents the physical layer for the
-electrical signals for DSI.
-
-Required properties:
-- compatible: Must be:
- - "fsl,imx8mq-mipi-dphy"
-- clocks: Must contain an entry for each entry in clock-names.
-- clock-names: Must contain the following entries:
- - "phy_ref": phandle and specifier referring to the DPHY ref clock
-- reg: the register range of the PHY controller
-- #phy-cells: number of cells in PHY, as defined in
- Documentation/devicetree/bindings/phy/phy-bindings.txt
- this must be <0>
-
-Optional properties:
-- power-domains: phandle to power domain
-
-Example:
- dphy: dphy@30a0030 {
- compatible = "fsl,imx8mq-mipi-dphy";
- clocks = <&clk IMX8MQ_CLK_DSI_PHY_REF>;
- clock-names = "phy_ref";
- reg = <0x30a00300 0x100>;
- power-domains = <&pd_mipi0>;
- #phy-cells = <0>;
-};
diff --git a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml
b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml
new file mode 100644
index ..c34f2e6d6bd5
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mixel,mipi-dsi-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mixel DSI PHY for i.MX8
+
+maintainers:
+ - Guido Günther
+
+description: |
+ The Mixel MIPI-DSI PHY IP block is e.g. found on i.MX8 platforms (along the
+ MIPI-DSI IP from Northwest Logic). It represents the physical layer for the
+ electrical signals for DSI.
+
+properties:
+ compatible:
+enum:
+ - fsl,imx8mq-mipi-dphy
+
+ reg:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+const: phy_ref
+
+ assigned-clocks:
+maxItems: 1
+
+ assigned-clock-parents:
+maxItems: 1
+
+ assigned-clock-rates:
+maxItems: 1
+
+ "#phy-cells":
+const: 0
+
+ power-domains:
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - assigned-clocks
+ - assigned-clock-parents
+ - assigned-clock-rates
+ - "#phy-cells"
+ - power-domains
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+dphy: dphy@30a0030 {
+compatible = "fsl,imx8mq-mipi-dphy";
+reg = <0x30a00300 0x100>;
+clocks = <&clk IMX8MQ_CLK_DSI_PHY_REF>;
+clock-names = "phy_ref";
+assigned-clocks = <&clk IMX8MQ_CLK_DSI_PHY_REF>;
+assigned-clock-parents = <&clk IMX8MQ_VIDEO_PLL1_OUT>;
+assigned-clock-rates = <2400>;
+#phy-cells = <0>;
+power-domains = <&pgc_mipi>;
+};
--
2.25.1
[PATCH v8 4/5] dt-bindings: phy: mixel: mipi-dsi-phy: Add Mixel combo PHY support for i.MX8qxp
Add support for Mixel MIPI DPHY + LVDS PHY combo IP as found on Freescale i.MX8qxp SoC. Cc: Guido Günther Cc: Kishon Vijay Abraham I Cc: Vinod Koul Cc: Rob Herring Cc: NXP Linux Team Reviewed-by: Rob Herring Reviewed-by: Guido Günther Signed-off-by: Liu Ying --- v7->v8: * No change. v6->v7: * No change. v5->v6: * No change. v4->v5: * No change. v3->v4: * Add Rob's and Guido's R-b tags. v2->v3: * No change. v1->v2: * Add the binding for i.MX8qxp Mixel combo PHY based on the converted binding. (Guido) .../bindings/phy/mixel,mipi-dsi-phy.yaml | 41 +-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml index c34f2e6d6bd5..786cfd71cb7e 100644 --- a/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml +++ b/Documentation/devicetree/bindings/phy/mixel,mipi-dsi-phy.yaml @@ -14,10 +14,14 @@ description: | MIPI-DSI IP from Northwest Logic). It represents the physical layer for the electrical signals for DSI. + The Mixel PHY IP block found on i.MX8qxp is a combo PHY that can work + in either MIPI-DSI PHY mode or LVDS PHY mode. + properties: compatible: enum: - fsl,imx8mq-mipi-dphy + - fsl,imx8qxp-mipi-dphy reg: maxItems: 1 @@ -40,6 +44,11 @@ properties: "#phy-cells": const: 0 + fsl,syscon: +$ref: /schemas/types.yaml#/definitions/phandle +description: | + A phandle which points to Control and Status Registers(CSR) module. + power-domains: maxItems: 1 @@ -48,12 +57,38 @@ required: - reg - clocks - clock-names - - assigned-clocks - - assigned-clock-parents - - assigned-clock-rates - "#phy-cells" - power-domains +allOf: + - if: + properties: +compatible: + contains: +const: fsl,imx8mq-mipi-dphy +then: + properties: +fsl,syscon: false + + required: +- assigned-clocks +- assigned-clock-parents +- assigned-clock-rates + + - if: + properties: +compatible: + contains: +const: fsl,imx8qxp-mipi-dphy +then: + properties: +assigned-clocks: false +assigned-clock-parents: false +assigned-clock-rates: false + + required: +- fsl,syscon + additionalProperties: false examples: -- 2.25.1
[PATCH v8 5/5] phy: freescale: phy-fsl-imx8-mipi-dphy: Add i.MX8qxp LVDS PHY mode support
i.MX8qxp SoC embeds a Mixel MIPI DPHY + LVDS PHY combo which supports
either a MIPI DSI display or a LVDS display. The PHY mode is controlled
by SCU firmware and the driver would call a SCU firmware function to
configure the PHY mode. The single LVDS PHY has 4 data lanes to support
a LVDS display. Also, with a master LVDS PHY and a slave LVDS PHY, they
may work together to support a LVDS display with 8 data lanes(usually, dual
LVDS link display). Note that this patch supports the LVDS PHY mode only
for the i.MX8qxp Mixel combo PHY, i.e., the MIPI DPHY mode is yet to be
supported, so for now error would be returned from ->set_mode() if MIPI
DPHY mode is passed over to it for the combo PHY.
Cc: Guido Günther
Cc: Robert Chiras
Cc: Kishon Vijay Abraham I
Cc: Vinod Koul
Cc: Shawn Guo
Cc: Sascha Hauer
Cc: Pengutronix Kernel Team
Cc: Fabio Estevam
Cc: NXP Linux Team
Reviewed-by: Guido Günther
Signed-off-by: Liu Ying
---
v7->v8:
* No change.
v6->v7:
* Use marco instead of magic number for CCM and CA values.
* Suppress 'checkpatch --strict' warnings.
* Check !opts in mixel_dphy_configure().
v5->v6:
* No change.
v4->v5:
* No change.
v3->v4:
* Add Guido's R-b tag.
v2->v3:
* Improve readability of mixel_dphy_set_mode(). (Guido)
v1->v2:
* Print invalid PHY mode in dmesg. (Guido)
.../phy/freescale/phy-fsl-imx8-mipi-dphy.c| 276 +-
1 file changed, 265 insertions(+), 11 deletions(-)
diff --git a/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c
b/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c
index a95572b397ca..e625b32889bf 100644
--- a/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c
+++ b/drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c
@@ -4,17 +4,33 @@
* Copyright 2019 Purism SPC
*/
+#include
#include
#include
#include
+#include
+#include
#include
#include
+#include
#include
#include
#include
#include
#include
#include
+#include
+
+/* Control and Status Registers(CSR) */
+#define PHY_CTRL 0x00
+#define CCM_MASK GENMASK(7, 5)
+#define CCM(n)FIELD_PREP(CCM_MASK, (n))
+#define CCM_1_2V 0x5
+#define CA_MASK GENMASK(4, 2)
+#define CA_3_51MA 0x4
+#define CA(n) FIELD_PREP(CA_MASK, (n))
+#define RFB BIT(1)
+#define LVDS_EN BIT(0)
/* DPHY registers */
#define DPHY_PD_DPHY 0x00
@@ -55,8 +71,15 @@
#define PWR_ON 0
#define PWR_OFF1
+#define MIN_VCO_FREQ 64000
+#define MAX_VCO_FREQ 15
+
+#define MIN_LVDS_REFCLK_FREQ 2400
+#define MAX_LVDS_REFCLK_FREQ 15000
+
enum mixel_dphy_devtype {
MIXEL_IMX8MQ,
+ MIXEL_IMX8QXP,
};
struct mixel_dphy_devdata {
@@ -65,6 +88,7 @@ struct mixel_dphy_devdata {
u8 reg_rxlprp;
u8 reg_rxcdrp;
u8 reg_rxhs_settle;
+ bool is_combo; /* MIPI DPHY and LVDS PHY combo */
};
static const struct mixel_dphy_devdata mixel_dphy_devdata[] = {
@@ -74,6 +98,10 @@ static const struct mixel_dphy_devdata mixel_dphy_devdata[]
= {
.reg_rxlprp = 0x40,
.reg_rxcdrp = 0x44,
.reg_rxhs_settle = 0x48,
+ .is_combo = false,
+ },
+ [MIXEL_IMX8QXP] = {
+ .is_combo = true,
},
};
@@ -95,8 +123,12 @@ struct mixel_dphy_cfg {
struct mixel_dphy_priv {
struct mixel_dphy_cfg cfg;
struct regmap *regmap;
+ struct regmap *lvds_regmap;
struct clk *phy_ref_clk;
const struct mixel_dphy_devdata *devdata;
+ struct imx_sc_ipc *ipc_handle;
+ bool is_slave;
+ int id;
};
static const struct regmap_config mixel_dphy_regmap_config = {
@@ -317,7 +349,8 @@ static int mixel_dphy_set_pll_params(struct phy *phy)
return 0;
}
-static int mixel_dphy_configure(struct phy *phy, union phy_configure_opts
*opts)
+static int
+mixel_dphy_configure_mipi_dphy(struct phy *phy, union phy_configure_opts *opts)
{
struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
struct mixel_dphy_cfg cfg = { 0 };
@@ -345,15 +378,126 @@ static int mixel_dphy_configure(struct phy *phy, union
phy_configure_opts *opts)
return 0;
}
+static int
+mixel_dphy_configure_lvds_phy(struct phy *phy, union phy_configure_opts *opts)
+{
+ struct mixel_dphy_priv *priv = phy_get_drvdata(phy);
+ struct phy_configure_opts_lvds *lvds_opts = &opts->lvds;
+ unsigned long data_rate;
+ unsigned long fvco;
+ u32 rsc;
+ u32 co;
+ int ret;
+
+ priv->is_slave = lvds_opts->is_slave;
+
+ /* LVDS interface pins */
+ regmap_write(priv->lvds_regmap, PHY_CTRL,
+CCM(CCM_1_2V) | CA(CA_3_51MA) | RFB);
+
+ /* enable MODE8 only for slave LVDS PHY */
+ rsc = priv->id ? IMX_SC_R_MIPI_1 : IMX_SC_R_MIPI_0;
+ ret = imx_sc_misc_set_control(priv->ipc_ha
RE: [PATCH v2 0/7] Add RZ/G2L Display support
Hi All,
Gentle ping, Are we happy with this patch series?
Cheers,
Biju
> Subject: [PATCH v2 0/7] Add RZ/G2L Display support
>
> RZ/G2L LCD controller composed of Frame compression Processor(FCPVD),
> Video signal processor (VSPD) and Display unit(DU). The output of LCDC is
> connected to Display parallel interface and MIPI link video interface.
>
> This patch series aims to add basic display support on RZ/G2L SMARC EVK
> platform. The output from DSI is connected to ADV7535.
>
> The DU controller is similar to R-Car as it is connected to VSPD, so
> reusing most of R-Car code with new CRTC driver specific to RZ/G2L
>
> v1->v2:
> * Based on [1], all references to 'rzg2l_lcdc' replaced with 'rzg2l_du'
> * Updated commit description for bindings
> * Removed LCDC references from bindings
> * Changed clock name from du.0->aclk from bindings
> * Changed reset name from du.0->du from bindings
> * Replaced crtc_helper_funcs->rcar_crtc_helper_funcs
> * Updated macro DRM_RZG2L_LCDC->DRM_RZG2L_DU
> * Replaced rzg2l-lcdc-drm->rzg2l-du-drm
> * Added forward declaration for struct reset_control
>
> [1]
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.kernel.org%2Fproject%2Flinux-renesas-soc%2Fpatch%2F20220312084205.31462-
> 2-
> biju.das.jz%40bp.renesas.com%2F&data=04%7C01%7Cbiju.das.jz%40bp.renesa
> s.com%7C619fcf75486d47a9954408da074e6e1b%7C53d82571da1947e49cb4625a166a4a2
> a%7C0%7C0%7C637830330678043182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=3zlHOfdKJ
> XLmDLGaMbbw%2BDUxQreKIEtvGUHNSuukDmg%3D&reserved=0
>
> RFC->v1:
> * Changed minItems->maxItems for renesas,vsps.
> * Added RZ/G2L LCDC driver with special handling for CRTC reusing
>most of RCar DU code
> * Fixed the comments for num_rpf from rpf's->RPFs/ and vsp->VSP.
> RFC:
>
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.kernel.org%2Fproject%2Flinux-renesas-soc%2Fpatch%2F20220112174612.10773-
> 18-
> biju.das.jz%40bp.renesas.com%2F&data=04%7C01%7Cbiju.das.jz%40bp.renesa
> s.com%7C619fcf75486d47a9954408da074e6e1b%7C53d82571da1947e49cb4625a166a4a2
> a%7C0%7C0%7C637830330678043182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=SXadiMRg%
> 2Fw%2Fnt3R6K02Zke67CSFqIQtt34si2RCqyH0%3D&reserved=0
>
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.kernel.org%2Fproject%2Flinux-renesas-soc%2Fpatch%2F20220112174612.10773-
> 12-
> biju.das.jz%40bp.renesas.com%2F&data=04%7C01%7Cbiju.das.jz%40bp.renesa
> s.com%7C619fcf75486d47a9954408da074e6e1b%7C53d82571da1947e49cb4625a166a4a2
> a%7C0%7C0%7C637830330678043182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=uRkp8himf
> 53knLtbWBxfRa4HGY3SxmyLT5FBrpmFtqg%3D&reserved=0
>
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.kernel.org%2Fproject%2Flinux-renesas-soc%2Fpatch%2F20220112174612.10773-
> 13-
> biju.das.jz%40bp.renesas.com%2F&data=04%7C01%7Cbiju.das.jz%40bp.renesa
> s.com%7C619fcf75486d47a9954408da074e6e1b%7C53d82571da1947e49cb4625a166a4a2
> a%7C0%7C0%7C637830330678043182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=MQAEyp28C
> rxHTvdHtarXlO6j0CkpCXZuqVHcbNWkXYI%3D&reserved=0
>
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwor
> k.kernel.org%2Fproject%2Flinux-renesas-soc%2Fpatch%2F20220112174612.10773-
> 19-
> biju.das.jz%40bp.renesas.com%2F&data=04%7C01%7Cbiju.das.jz%40bp.renesa
> s.com%7C619fcf75486d47a9954408da074e6e1b%7C53d82571da1947e49cb4625a166a4a2
> a%7C0%7C0%7C637830330678043182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=1y%2Bd5Yb
> UoXnMZL97%2F4LTcG8IDtze%2FW%2BwzHRXBEbUgSw%3D&reserved=0
>
> Biju Das (7):
> dt-bindings: display: renesas,du: Document r9a07g044l bindings
> drm: rcar-du: Add num_rpf to struct rcar_du_device_info
> drm: rcar-du: Add max_width and max_height to struct
> rcar_du_device_info
> drm: rcar-du: Move rcar_du_output_name() to rcar_du_common.c
> drm: rcar-du: Factorise rcar_du_{atomic_check,modeset_init}
> drm: rcar-du: Factorise
> rcar_du_vsp{complete,enable,plane_atomic_check}
> drm: rcar-du: Add RZ/G2L DU Support
>
> .../bindings/display/renesas,du.yaml | 54 ++
> drivers/gpu/drm/rcar-du/Kconfig | 18 +-
> drivers/gpu/drm/rcar-du/Makefile | 13 +
> drivers/gpu/drm/rcar-du/rcar_du_common.c | 30 +
> drivers/gpu/drm/rcar-du/rcar_du_crtc.h| 8 +
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 100 ++-
> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 31 +
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 23 +-
> drivers/gpu/drm/rcar-du/rcar_du_plane.h | 12 +
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 18 +-
>
RE: [PATCH v2 0/2] Add RZ/G2L DSI driver
Hi All, Gentle ping. Are we happy with this patch series? Please let me know. Cheers, Biju > Subject: [PATCH v2 0/2] Add RZ/G2L DSI driver > > This patch series aims to support the MIPI DSI encoder found in the RZ/G2L > SoC. It currently supports DSI mode only. > > This unit supports MIPI Alliance Specification for Display Serial > Interface (DSI) Specification. This unit provides a solution for > transmitting MIPI DSI compliant digital video and packets. Normative > References are below. > * MIPI Alliance Specification for Display Serial Interface Version 1.3.1 > * MIPI Alliance Specification for D-PHY Version 2.1 > > The following are key features of this unit. > > * 1 channel > * The number of Lane: 4-lane > * Support up to Full HD (1920 × 1080), 60 fps (RGB888) > * Maximum Bandwidth: 1.5 Gbps per lane > * Support Output Data Format: RGB666 / RGB888 > > v1->v2: > * Added full path for dsi-controller.yaml > * Modeled DSI + D-PHY as single block and updated reg property > * Fixed typo D_PHY->D-PHY > * Updated description > * Added interrupts and interrupt-names and updated the example > * Driver rework based on dt-binding changes (DSI + D-PHY) as single block > * Replaced link_mmio and phy_mmio with mmio in struct rzg2l_mipi_dsi > * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write >and rzg2l_mipi_dsi_link_write > * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read > RFC->v1: > * Added a ref to dsi-controller.yaml. > * Added "depends on ARCH_RENESAS || COMPILE_TEST" on KCONFIG >and dropped DRM as it is implied by DRM_BRIDGE > * Used devm_reset_control_get_exclusive() for reset handle > * Removed bool hsclkmode from struct rzg2l_mipi_dsi > * Added error check for pm, using pm_runtime_resume_and_get() instead of >pm_runtime_get_sync() > * Added check for unsupported formats in rzg2l_mipi_dsi_host_attach() > * Avoided read-modify-write stopping hsclock > * Used devm_platform_ioremap_resource for resource allocation > * Removed unnecessary assert call from probe and remove. > * wrap the line after the PTR_ERR() in probe() > * Updated reset failure messages in probe > * Fixed the typo arstc->prstc > * Made hex constants to lower case. > RFC: > * > > Biju Das (2): > dt-bindings: display: bridge: Document RZ/G2L MIPI DSI TX bindings > drm: rcar-du: Add RZ/G2L DSI driver > > .../bindings/display/bridge/renesas,dsi.yaml | 175 + > drivers/gpu/drm/rcar-du/Kconfig | 8 + > drivers/gpu/drm/rcar-du/Makefile | 1 + > drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c | 686 ++ > drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h | 149 > 5 files changed, 1019 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml > create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c > create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h > > -- > 2.17.1
[PATCH] drm/i915: Remove intel_gvt_init_host declaration
this function has been deleted since commit 9bdb073464d6 ("drm/i915/gvt:
Change KVMGT as self load module"), remove the deprecated function
declaration.
Signed-off-by: Cong Liu
---
drivers/gpu/drm/i915/intel_gvt.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index d7d3fb6186fd..daaf0957ebbc 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -31,7 +31,6 @@ int intel_gvt_init(struct drm_i915_private *dev_priv);
void intel_gvt_driver_remove(struct drm_i915_private *dev_priv);
int intel_gvt_init_device(struct drm_i915_private *dev_priv);
void intel_gvt_clean_device(struct drm_i915_private *dev_priv);
-int intel_gvt_init_host(void);
void intel_gvt_sanitize_options(struct drm_i915_private *dev_priv);
void intel_gvt_resume(struct drm_i915_private *dev_priv);
#else
--
2.25.1
[PULL] drm-misc-next
Hi Daniel, Dave, Here's this week drm-misc-next PR Maxime drm-misc-next-2022-04-14: drm-misc-next for $kernel-version: UAPI Changes: Cross-subsystem Changes: - fbcon: various loading and locking cleanups Core Changes: - buddy: Check for page size alignment at allocation - dma-buf: Add dma_resv_usage enum, new usage for kernel submissions, memory management submissions, documentation improvements, Driver Changes: - Switch drivers to drm_mode_init, drm_mode_copy - amdgpu: Add drm buddy allocator support - bridge: Add DDC enable gpio support to display-connector, convert icn6211 to regmap - ingenic: HDMI support - panels: Support for SINO WEALTH SH1106 OLED display, simplify mode parsing in panel-lvds - sti: spelling fixes, bridge list iteration fixes - vc4: Writeback fixes and improvements The following changes since commit c8d4c18bfbc4ab467188dbe45cc8155759f49d9e: dma-buf/drivers: make reserving a shared slot mandatory v4 (2022-04-06 17:38:25 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2022-04-14 for you to fetch changes up to 9d79799193b728b62c9899d931b5009da1f89b67: fbcon: Fix delayed takeover locking (2022-04-13 22:54:48 +0200) drm-misc-next for $kernel-version: UAPI Changes: Cross-subsystem Changes: - fbcon: various loading and locking cleanups Core Changes: - buddy: Check for page size alignment at allocation - dma-buf: Add dma_resv_usage enum, new usage for kernel submissions, memory management submissions, documentation improvements, Driver Changes: - Switch drivers to drm_mode_init, drm_mode_copy - amdgpu: Add drm buddy allocator support - bridge: Add DDC enable gpio support to display-connector, convert icn6211 to regmap - ingenic: HDMI support - panels: Support for SINO WEALTH SH1106 OLED display, simplify mode parsing in panel-lvds - sti: spelling fixes, bridge list iteration fixes - vc4: Writeback fixes and improvements Arunpravin Paneer Selvam (2): drm/amdgpu: add drm buddy support to amdgpu drm: add a check to verify the size alignment Changcheng Deng (1): fbcon: use min() to make code cleaner Chen-Yu Tsai (4): dt-bindings: vendor-prefixes: Add prefix for SINO WEALTH Eletronics Ltd. dt-bindings: display: ssd1307fb: Add entry for SINO WEALTH SH1106 drm/ssd130x: Support page addressing mode drm/ssd130x: Add support for SINO WEALTH SH1106 Christian König (16): dma-buf: add enum dma_resv_usage v4 dma-buf: specify usage while adding fences to dma_resv obj v7 dma-buf & drm/amdgpu: remove dma_resv workaround dma-buf: add DMA_RESV_USAGE_KERNEL v3 drm/amdgpu: use DMA_RESV_USAGE_KERNEL drm/radeon: use DMA_RESV_USAGE_KERNEL RDMA: use DMA_RESV_USAGE_KERNEL dma-buf: add DMA_RESV_USAGE_BOOKKEEP v3 dma-buf: wait for map to complete for static attachments drm/i915: drop bo->moving dependency drm/ttm: remove bo->moving dma-buf: drop seq count based update seqlock: drop seqcount_ww_mutex_t futex: add missing rtmutex.h include drm/ttm: fix logic inversion in ttm_eu_reserve_buffers drm/ttm: fix kerneldoc for ttm_lru_bulk_move Colin Ian King (1): drm: sti: fix spelling mistake: rejec -> rejection Daniel Vetter (18): fbcon: delete a few unneeded forward decl fbcon: Move fbcon_bmove(_rec) functions fbcon: Introduce wrapper for console->fb_info lookup fbcon: delete delayed loading code fbdev/sysfs: Fix locking fbcon: Use delayed work for cursor fbcon: Replace FBCON_FLAGS_INIT with a boolean fb: Delete fb_info->queue fbcon: Extract fbcon_open/release helpers fbcon: Ditch error handling for con2fb_release_oldinfo fbcon: move more common code into fb_open() fbcon: use lock_fb_info in fbcon_open/release fbcon: Consistently protect deferred_takeover with console_lock() fbcon: Move console_lock for register/unlink/unregister fbcon: Move more code into fbcon_release fbcon: untangle fbcon_exit fbcon: Maintain a private array of fb_info fbcon: Fix delayed takeover locking H. Nikolaus Schaller (1): drm/bridge: display-connector: add ddc-en gpio support Javier Martinez Canillas (1): drm/vc4: Use newer fence API properly to fix build errors Jiasheng Jiang (1): drm: mali-dp: potential dereference of null pointer Julia Lawall (1): drm/sti: fix typos in comments Liviu Dudau (1): drm/komeda: return early if drm_universal_plane_init() fails. Marek Vasut (3): drm: bridge: icn6211: Convert to regmap drm/panel: lvds: Simplify mode parsing drm/panel: lvds: Use bus_flags from DT panel-timing property Matthew Auld (4): drm/i915: fix broken build
Re: [PATCHv4] drm/amdgpu: disable ASPM on Intel Alder Lake based systems
[Cc: -kernel test robot ]
Dear Alex, dear Richard,
Am 13.04.22 um 15:00 schrieb Alex Deucher:
On Wed, Apr 13, 2022 at 3:43 AM Paul Menzel wrote:
Thank you for sending out v4.
Am 12.04.22 um 23:50 schrieb Richard Gong:
Active State Power Management (ASPM) feature is enabled since kernel 5.14.
There are some AMD GFX cards (such as WX3200 and RX640) that won't work
with ASPM-enabled Intel Alder Lake based systems. Using these GFX cards as
video/display output, Intel Alder Lake based systems will hang during
suspend/resume.
I am still not clear, what “hang during suspend/resume” means. I guess
suspending works fine? During resume (S3 or S0ix?), where does it hang?
The system is functional, but there are only display problems?
The issue was initially reported on one system (Dell Precision 3660 with
BIOS version 0.14.81), but was later confirmed to affect at least 4 Alder
Lake based systems.
Add extra check to disable ASPM on Intel Alder Lake based systems.
Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default")
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1885
Reported-by: kernel test robot
This tag is a little confusing. Maybe clarify that it was for an issue
in a previous patch iteration?
Signed-off-by: Richard Gong
---
v4: s/CONFIG_X86_64/CONFIG_X86
enhanced check logic
v3: s/intel_core_asom_chk/aspm_support_quirk_check
correct build error with W=1 option
v2: correct commit description
move the check from chip family to problematic platform
---
drivers/gpu/drm/amd/amdgpu/vi.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index 039b90cdc3bc..b33e0a9bee65 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -81,6 +81,10 @@
#include "mxgpu_vi.h"
#include "amdgpu_dm.h"
+#if IS_ENABLED(CONFIG_X86)
+#include
+#endif
+
#define ixPCIE_LC_L1_PM_SUBSTATE0x100100C6
#define PCIE_LC_L1_PM_SUBSTATE__LC_L1_SUBSTATES_OVERRIDE_EN_MASK
0x0001L
#define PCIE_LC_L1_PM_SUBSTATE__LC_PCI_PM_L1_2_OVERRIDE_MASK
0x0002L
@@ -1134,13 +1138,24 @@ static void vi_enable_aspm(struct amdgpu_device *adev)
WREG32_PCIE(ixPCIE_LC_CNTL, data);
}
+static bool aspm_support_quirk_check(void)
+{
+ if (IS_ENABLED(CONFIG_X86)) {
+ struct cpuinfo_x86 *c = &cpu_data(0);
+
+ return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
+ }
+
+ return true;
+}
+
static void vi_program_aspm(struct amdgpu_device *adev)
{
u32 data, data1, orig;
bool bL1SS = false;
bool bClkReqSupport = true;
- if (!amdgpu_device_should_use_aspm(adev))
+ if (!amdgpu_device_should_use_aspm(adev) || !aspm_support_quirk_check())
return;
Can users still forcefully enable ASPM with the parameter `amdgpu.aspm`?
if (adev->flags & AMD_IS_APU ||
If I remember correctly, there were also newer cards, where ASPM worked
with Intel Alder Lake, right? Can only the problematic generations for
WX3200 and RX640 be excluded from ASPM?
This patch only disables it for the generation that was problematic.
Could that please be made clear in the commit message summary, and message?
Loosely related, is there a public (or internal issue) to analyze how to
get ASPM working for VI generation devices with Intel Alder Lake?
Kind regards,
Paul
Re: [PATCH v7 1/5] drm/bridge: nwl-dsi: Set PHY mode in nwl_dsi_mode_set()
Hi,
On Thu, Apr 14, 2022 at 02:48:54PM +0800, Liu Ying wrote:
> The Northwest Logic MIPI DSI host controller embedded in i.MX8qxp
> works with a Mixel MIPI DPHY + LVDS PHY combo to support either
> a MIPI DSI display or a LVDS display. So, this patch calls
> phy_set_mode() from nwl_dsi_mode_set() to set PHY mode to MIPI DPHY
> explicitly.
>
> Cc: Guido Günther
> Cc: Robert Chiras
> Cc: Martin Kepplinger
> Cc: Andrzej Hajda
> Cc: Neil Armstrong
> Cc: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: NXP Linux Team
> Signed-off-by: Liu Ying
> ---
> v6->v7:
> * No change.
>
> v5->v6:
> * Rebase the series upon v5.17-rc1.
> * Set PHY mode in ->mode_set() instead of ->pre_enable() in the nwl-dsi
> bridge driver due to the rebase.
> * Drop Guido's R-b tag due to the rebase.
>
> v4->v5:
> * No change.
>
> v3->v4:
> * No change.
>
> v2->v3:
> * No change.
>
> v1->v2:
> * Add Guido's R-b tag.
>
> drivers/gpu/drm/bridge/nwl-dsi.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c
> b/drivers/gpu/drm/bridge/nwl-dsi.c
> index d5945501a5ee..85bab7372af1 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -666,6 +666,12 @@ static int nwl_dsi_mode_set(struct nwl_dsi *dsi)
> return ret;
> }
>
> + ret = phy_set_mode(dsi->phy, PHY_MODE_MIPI_DPHY);
> + if (ret < 0) {
> + DRM_DEV_ERROR(dev, "Failed to set DSI phy mode: %d\n", ret);
> + goto uninit_phy;
> + }
> +
> ret = phy_configure(dsi->phy, phy_cfg);
> if (ret < 0) {
> DRM_DEV_ERROR(dev, "Failed to configure DSI phy: %d\n", ret);
> --
> 2.25.1
>
Reviewed-by: Guido Günther
Cheers,
-- Guido
Re: commit 15512021eb3975a8c2366e3883337e252bb0eee5 causes white spots in console screens
On Thu, Apr 14, 2022 at 08:33:35AM +0200, François Valenduc wrote:
> Le 14/04/22 à 08:31, Lisovskiy, Stanislav a écrit :
> > On Wed, Apr 13, 2022 at 08:12:20PM +0300, Jani Nikula wrote:
> > > On Wed, 13 Apr 2022, François Valenduc wrote:
> > > > Commit 15512021eb3975a8c2366e3883337e252bb0eee5
> > > > (15512021eb3975a8c2366e3883337e252bb0eee5) causes a lof of white spots
> > > > to appears on the right upper corner of all console screens (see
> > > > https://drive.google.com/file/d/13GabEvOIKSAj5yox6ybAZEDu3Ixncw5Q/view).
> > > > git-bisect shows that this is the offending commit and if I revert it,
> > > > the problem goes away. The problem still occurs with kernel 5.18-rc2 and
> > > > to all stable trees where it was applied.
> > > > Can somebody explains what happens ?
> > > >
> > > > The video card is the following: VGA compatible controller: Intel
> > > > Corporation WhiskeyLake-U GT2 [UHD Graphics 620] (rev 02) (prog-if 00
> > > > [VGA controller])
> > > >
> > > > Please tell me if you need more info.
> > > That's commit 15512021eb39 ("drm/i915: Workaround broken BIOS DBUF
> > > configuration on TGL/RKL"), adding Cc's.
> > >
> > > Please file a report at fdo gitlab [1] and attach dmesg etc. there.
> > That commit looks like it is just disabling all the planes, if wrong
> > dbuf/wm configuration is detected.
> > However it should do that only once during boot as I understand.
> >
> > Are you sure that is exactly this commit which is causing this?
> > Does the issue appear always?
> >
> > Ville Syrjälä, thoughts?
> >
> > Stan
> >
> > > Thanks,
> > > Jani.
> > >
> > >
> > > [1] https://gitlab.freedesktop.org/drm/intel/wikis/How-to-file-i915-bugs
> > >
> > >
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
>
> As I said, git-bisect shows it's the offending commit and if I revert it,
> the problem doesn't happens. Otherwise, it always occurs.
>
> François
>
Does it just happen all the time or some steps/certain circumstances needed
for it to happen?
Only suspicion after looking briefly is that once suspend/resume is done
it might be messing something up. Just a quick guess..
Stan
Re: commit 15512021eb3975a8c2366e3883337e252bb0eee5 causes white spots in console screens
Le 14/04/22 à 10:03, Lisovskiy, Stanislav a écrit :
On Thu, Apr 14, 2022 at 08:33:35AM +0200, François Valenduc wrote:
Le 14/04/22 à 08:31, Lisovskiy, Stanislav a écrit :
On Wed, Apr 13, 2022 at 08:12:20PM +0300, Jani Nikula wrote:
On Wed, 13 Apr 2022, François Valenduc wrote:
Commit 15512021eb3975a8c2366e3883337e252bb0eee5
(15512021eb3975a8c2366e3883337e252bb0eee5) causes a lof of white spots
to appears on the right upper corner of all console screens (see
https://drive.google.com/file/d/13GabEvOIKSAj5yox6ybAZEDu3Ixncw5Q/view).
git-bisect shows that this is the offending commit and if I revert it,
the problem goes away. The problem still occurs with kernel 5.18-rc2 and
to all stable trees where it was applied.
Can somebody explains what happens ?
The video card is the following: VGA compatible controller: Intel
Corporation WhiskeyLake-U GT2 [UHD Graphics 620] (rev 02) (prog-if 00
[VGA controller])
Please tell me if you need more info.
That's commit 15512021eb39 ("drm/i915: Workaround broken BIOS DBUF
configuration on TGL/RKL"), adding Cc's.
Please file a report at fdo gitlab [1] and attach dmesg etc. there.
That commit looks like it is just disabling all the planes, if wrong
dbuf/wm configuration is detected.
However it should do that only once during boot as I understand.
Are you sure that is exactly this commit which is causing this?
Does the issue appear always?
Ville Syrjälä, thoughts?
Stan
Thanks,
Jani.
[1] https://gitlab.freedesktop.org/drm/intel/wikis/How-to-file-i915-bugs
--
Jani Nikula, Intel Open Source Graphics Center
As I said, git-bisect shows it's the offending commit and if I revert it,
the problem doesn't happens. Otherwise, it always occurs.
François
Does it just happen all the time or some steps/certain circumstances needed
for it to happen?
Only suspicion after looking briefly is that once suspend/resume is done
it might be messing something up. Just a quick guess..
Stan
It occurs permantently as soon as I boot my computer.
Re: [PATCH v10 12/24] drm/rockchip: dw_hdmi: drop mode_valid hook
On Mon, Apr 11, 2022 at 07:00:51PM +0200, Alex Bee wrote:
> Am 11.04.22 um 09:53 schrieb Sascha Hauer:
> > On Sun, Apr 10, 2022 at 01:31:23PM +0200, Alex Bee wrote:
> >> Am 08.04.22 um 13:22 schrieb Sascha Hauer:
> >>> The driver checks if the pixel clock of the given mode matches an entry
> >>> in the mpll config table. The frequencies in the mpll table are meant as
> >>> a frequency range up to which the entry works, not as a frequency that
> >>> must match the pixel clock. The downstream Kernel also does not have
> >>> this check, so drop it to allow for more display resolutions.
> >>>
> >>> Signed-off-by: Sascha Hauer
> >>> ---
> >>>
> >> You're correct: That frequency is meant to be greater or equal. But I'm
> >> not sure if it makes sense to completely drop it - what happens for
> >> clocks rates > 600 MHz which might be supported by later generation
> >> sinks (HDMI 2.1 or later)?
> >> As these are not supported by the IPs/PHYs currently supported by that
> >> driver a reason a simple
> >>
> >> int i;
> >>
> >>
> >>
> >> for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
> >>
> >> - if (pclk == mpll_cfg[i].mpixelclock) {
> >>
> >> + if (pclk >= mpll_cfg[i].mpixelclock) {
> >
> > Should be <=
>
> Sure, sorry 'bout that.
> >
> > No other user currently in the tree has this check.
> I dont't think that's true - it might not be checked against mpll table,
> but max pixelclocks are checked in meson_encoder_hdmi_mode_valid for
> amlogic, in sun8i_dw_hdmi_mode_valid_a83t and
> sun8i_dw_hdmi_mode_valid_h6 for allwinner and rcar_hdmi_mode_valid for
> rcar platform. There is no other point in rockchip dw-hdmi platform
> driver where this is currently checked.
>
> > hdmi_phy_configure_dwc_hdmi_3d_tx() has this:
> >
> >>/* PLL/MPLL Cfg - always match on final entry */
> >>for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
> >>if (mpixelclock <= mpll_config->mpixelclock)
> >>break;
> >>
> >>for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
> >>if (mpixelclock <= curr_ctrl->mpixelclock)
> >>break;
> >>
> >>for (; phy_config->mpixelclock != ~0UL; phy_config++)
> >>if (mpixelclock <= phy_config->mpixelclock)
> >>break;
> >>
> >>if (mpll_config->mpixelclock == ~0UL ||
> >>curr_ctrl->mpixelclock == ~0UL ||
> >>phy_config->mpixelclock == ~0UL)
> >>return -EINVAL;
> >
> > This means we already have this check and others already in the generic
> > part of the dw-hdmi driver. Should we maybe do the above in
> > dw_hdmi_bridge_mode_valid() instead of doing it in the users?
> I guess that's not possible, due to the different kind of phys which are
> used for dw-hdmi. The checks you are refering to, are only done for dw
> hdmi tx phys, but not for "vendor phys" (like for RK3328 and RK3228, for
> example) - those have have own drivers which are handled only in dw-hdmi
> platform driver.
> Therefore this check should remain here for Rockchip also.
Ok, agreed. I'll keep the check for the next round and just replace
the == with <=.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding
Hi Samuel, for comparison, here is my submission for the IMX EPDC bindings: https://lore.kernel.org/linux-devicetree/20220206080016.796556-2-andr...@kemnade.info/ On Wed, 13 Apr 2022 17:19:02 -0500 Samuel Holland wrote: [...] we have sy7636a driver in kernel which should be suitable for powering a EPD and temperature measurement. So I would expect that to be > + io-channels: > +maxItems: 1 > +description: I/O channel for panel temperature measurement > + so how would I reference the hwmon/thermal(-zone) of the sy7636a here? > + panel-supply: > +description: Regulator supplying the panel's logic voltage > + > + power-domains: > +maxItems: 1 > + > + vcom-supply: > +description: Regulator supplying the panel's compensation voltage > + > + vdrive-supply: > +description: Regulator supplying the panel's gate and source drivers > + SY7636a has only one logical regulator in kernel for for the latter two. If we have a separate panel node, than maybe these regulators should go there as they belong to the panel as they are powering the panel and not the EBC. > + port: > +$ref: /schemas/graph.yaml#/properties/port > +description: OF graph port for the attached display panel > + In my approach for the IMX EPDC, (I will send a better commented one soon) I have no separate subnode to avoid messing with additional display parameters. Not sure what is really better here. > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - resets > + - reset-names > + - power-domains > + - panel-supply > + - vcom-supply > + - vdrive-supply If things differ how the different phyiscally existing regulators are mapped into logical ones (even the vdrive supply is still a bunch of physical regulators mapped into one logical one), then not everything can be required. Regards, Andreas
Re: commit 15512021eb3975a8c2366e3883337e252bb0eee5 causes white spots in console screens
On Thu, 14 Apr 2022, François Valenduc wrote:
> Le 14/04/22 à 10:03, Lisovskiy, Stanislav a écrit :
>> On Thu, Apr 14, 2022 at 08:33:35AM +0200, François Valenduc wrote:
>>> Le 14/04/22 à 08:31, Lisovskiy, Stanislav a écrit :
On Wed, Apr 13, 2022 at 08:12:20PM +0300, Jani Nikula wrote:
> On Wed, 13 Apr 2022, François Valenduc wrote:
>> Commit 15512021eb3975a8c2366e3883337e252bb0eee5
>> (15512021eb3975a8c2366e3883337e252bb0eee5) causes a lof of white spots
>> to appears on the right upper corner of all console screens (see
>> https://drive.google.com/file/d/13GabEvOIKSAj5yox6ybAZEDu3Ixncw5Q/view).
>> git-bisect shows that this is the offending commit and if I revert it,
>> the problem goes away. The problem still occurs with kernel 5.18-rc2 and
>> to all stable trees where it was applied.
>> Can somebody explains what happens ?
>>
>> The video card is the following: VGA compatible controller: Intel
>> Corporation WhiskeyLake-U GT2 [UHD Graphics 620] (rev 02) (prog-if 00
>> [VGA controller])
>>
>> Please tell me if you need more info.
> That's commit 15512021eb39 ("drm/i915: Workaround broken BIOS DBUF
> configuration on TGL/RKL"), adding Cc's.
>
> Please file a report at fdo gitlab [1] and attach dmesg etc. there.
That commit looks like it is just disabling all the planes, if wrong
dbuf/wm configuration is detected.
However it should do that only once during boot as I understand.
Are you sure that is exactly this commit which is causing this?
Does the issue appear always?
Ville Syrjälä, thoughts?
Stan
> Thanks,
> Jani.
>
>
> [1] https://gitlab.freedesktop.org/drm/intel/wikis/How-to-file-i915-bugs
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
>>> As I said, git-bisect shows it's the offending commit and if I revert it,
>>> the problem doesn't happens. Otherwise, it always occurs.
>>>
>>> François
>>>
>> Does it just happen all the time or some steps/certain circumstances needed
>> for it to happen?
>>
>> Only suspicion after looking briefly is that once suspend/resume is done
>> it might be messing something up. Just a quick guess..
>>
>> Stan
>
> It occurs permantently as soon as I boot my computer.
I'd appreciate that bug being filed, and the discussion moved there, so
all the details and history are in one place.
Thanks,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
Re: [RESEND][PATCH] omapdrm: fix missing check on list iterator
Hi,
On 14/04/2022 09:14, Xiaomeng Tong wrote:
The bug is here:
bus_flags = connector->display_info.bus_flags;
The list iterator 'connector-' will point to a bogus position containing
HEAD if the list is empty or no element is found. This case must
be checked before any use of the iterator, otherwise it will lead
to a invalid memory access.
To fix this bug, add an check. Use a new value 'iter' as the list
iterator, while use the old value 'connector' as a dedicated variable
to point to the found element.
Cc: sta...@vger.kernel.org
Fixes: ("drm/omap: Add support for drm_panel")
Signed-off-by: Xiaomeng Tong
---
drivers/gpu/drm/omapdrm/omap_encoder.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c
b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 4dd05bc732da..d648ab4223b1 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -76,14 +76,16 @@ static void omap_encoder_mode_set(struct drm_encoder
*encoder,
struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
struct omap_dss_device *output = omap_encoder->output;
struct drm_device *dev = encoder->dev;
- struct drm_connector *connector;
+ struct drm_connector *connector = NULL, *iter;
struct drm_bridge *bridge;
struct videomode vm = { 0 };
u32 bus_flags;
- list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
- if (connector->encoder == encoder)
+ list_for_each_entry(iter, &dev->mode_config.connector_list, head) {
+ if (iter->encoder == encoder) {
+ connector = iter;
break;
+ }
}
When does this bug happen? How do you get omap_encoder_mode_set() called
for an encoder with a connector that is not valid?
drm_display_mode_to_videomode(adjusted_mode, &vm);
@@ -106,8 +108,10 @@ static void omap_encoder_mode_set(struct drm_encoder
*encoder,
omap_encoder_update_videomode_flags(&vm, bus_flags);
}
- bus_flags = connector->display_info.bus_flags;
- omap_encoder_update_videomode_flags(&vm, bus_flags);
+ if (connector) {
+ bus_flags = connector->display_info.bus_flags;
+ omap_encoder_update_videomode_flags(&vm, bus_flags);
+ }
/* Set timings for all devices in the display pipeline. */
dss_mgr_set_timings(output, &vm);
How does this fix the issue? You just skip the lines that set up the
videomode, but then pass that videomode to dss_mgr_set_timings()...
Tomi
Re: [PATCH] drm/radeon: Add build directory to include path
Am 14.04.22 um 09:37 schrieb Michel Dänzer: On 2022-04-14 08:24, Christian König wrote: Am 13.04.22 um 18:14 schrieb Michel Dänzer: From: Michel Dänzer Fixes compile errors with out-of-tree builds, e.g. ../drivers/gpu/drm/radeon/r420.c:38:10: fatal error: r420_reg_safe.h: No such file or directory 38 | #include "r420_reg_safe.h" | ^ Well stuff like that usually points to a broken build environment. Just a separate build directory. Specifically, I'm hitting the errors with make -C build-amd64 M=drivers/gpu/drm Generated headers such as r420_reg_safe.h reside in the build directory, so source files in the source directory can't find them without an explicit search path. I'm trying to swap back into my brain how all of this used to work, but that's a really long time ago that I tried this as well. Are you saying that should get added automagically somehow? Yes, exactly that. I'm like 95% sure that used to work, but I don't know why exactly either. FWIW, this is pretty common in the kernel according to git grep. Maybe Alex or somebody else with some more background in the kernel Makefiles could jump in and help here. Christian.
Re: [RFC PATCH 00/16] drm/rockchip: Rockchip EBC ("E-Book Controller") display driver
On Wed, Apr 13, 2022 at 05:19:00PM -0500, Samuel Holland wrote:
> This series adds a DRM driver for the electrophoretic display controller
> found in a few different Rockchip SoCs, specifically the RK3566/RK3568
> variant[0] used by the PineNote tablet[1].
>
> This is my first real involvement with the DRM subsystem, so please let
> me know where I am misunderstanding things.
>
> This is now the second SoC-integrated EPD controller with a DRM driver
> submission -- the first one being the i.MX6 EPDC[2]. I want to thank
> Andreas for sending that series, and for his advice while writing this
> driver.
>
> One goal I have with sending this series is to discuss how to support
> EPDs more generally within the DRM subsystem, so the interfaces with
> panels and PMICs and waveform LUTs can be controller-independent.
>
> My understanding is that the i.MX6 EPDC series is at least partly based
> on the downstream vendor driver. This driver is a clean-sheet design for
> hardware with different (read: fewer) capabilities, so we took some
> different design paths, but we ran into many of the same sharp edges.
>
> Here are some of the areas I would like input on:
>
> Panel Lifecycle
> ===
> Panels use prepare/unprepare callbacks for their power supply. EPDs
> should only be powered up when the display contents are changed. Should
> the controller call both drm_panel_(un)prepare during each atomic update
> when the framebuffer is dirty?
>
> Similarly, panel enable/disable callbacks are tied to backlight state.
> For an EPD, it makes sense to have the backlight enabled while the panel
> is powered down (because the contents are static). Is it acceptable to
> call drm_panel_{en,dis}able while the panel is not prepared?
>
> With panel_bridge, the "normal" callback ordering is enforced, and tied
> to the atomic state, so neither of these is possible.
>
> As a result, neither the backlight nor the voltage regulators are tied
> to the panel. The panel's regulators are consumed by the EBC itself.
At least to manage the power state, that looks fairly similar to what we
have already to enter / exit from panel self refresh, so maybe we can
leverage that infrastructure?
And thus we would have something like enabling the backlight when we
prepare the panel, but only enable / disable the regulator when we exit
/ enter PSR mode?
Would that make sense?
> Panel Timing Parameters
> ===
> EPDs have more timing parameters than LCDs, and there are several
> different ways of labeling these parameters. See for example the timing
> diagrams on pp. 2237-2239 of the RK3568 TRM[0], the descriptions in the
> ED103TC2 panel datasheet[3], and the submitted EPDC bindings[2].
>
> Both the EPDC and EBC vendor drivers put all of the timing parameters in
> the controller's OF node. There is no panel device/node.
>
> I was able to squeeze everything needed for my specific case into a
> struct drm_display_mode (see patches 5 and 14), but I don't know if this
> is an acceptable use of those fields, or if will work with other
> controllers. Is adding more fields to drm_display_mode an option?
>
> See also the discussion of "dumb" LCD TCONs below.
Reading that datasheet and patch series, it's not clear to me whether
it's just a set of generic parameters for E-ink display, or if it's some
hardware specific representation of those timings.
Generally speaking, drm_display_mode is an approximation of what the
timings are. The exact clock rate for example will be widely different
between RGB, HDMI or MIPI-DSI (with or without burst). I think that as
long as you can derive a drm_display_mode from those parameters, and can
infer those parameters from a drm_display_mode, you can definitely reuse
it.
> Panel Connector Type / Media Bus Format
> ===
> The EBC supports either an 8-bit or 16-bit wide data bus, where each
> pair of data lines represents the source driver polarity (positive,
> negative, or neutral) for a pixel.
>
> The only effect of the data bus width is the number of pixels that are
> transferred per clock cycle. It has no impact on the number of possible
> grayscale levels.
>
> How does that translate to DRM_MODE_CONNECTOR_* or MEDIA_BUS_FMT_*?
We'll probably want a separate connector mode, but you could add a
parameter on the OF-graph endpoint to set the media bus format.
> Panel Reflection
>
> The ED103TC2 panel scans from right to left. Currently, there is no API
> or OF property to represent this. I can add something similar to
> drm_panel_orientation.
Yeah, leveraging DRM_MODE_REFLECT_X into something similar to
drm_panel_orientation makes sense
> Should this be exposed to userspace? It is acceptable for the kernel
> driver to flip the image when blitting from the framebuffer?
I'm not sure about whether or not we should expose it to userspace. I'd
say yes, but I'll leave it to others :)
> CRTC "active" and "enabled" states
> =
[PATCH] drm/bridge: anx7625: Fill in empty ELD when no connector
Speaker may share I2S with DP and .get_eld callback will be called when
speaker is playing. When HDMI wans't connected, the connector will be
null. Instead of return an error, fill in empty ELD.
Signed-off-by: Hsin-Yi Wang
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c
b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 6516f9570b86..f2bc30c98c77 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1932,14 +1932,14 @@ static int anx7625_audio_get_eld(struct device *dev,
void *data,
struct anx7625_data *ctx = dev_get_drvdata(dev);
if (!ctx->connector) {
- dev_err(dev, "connector not initial\n");
- return -EINVAL;
+ /* Pass en empty ELD if connector not available */
+ memset(buf, 0, len);
+ } else {
+ dev_dbg(dev, "audio copy eld\n");
+ memcpy(buf, ctx->connector->eld,
+ min(sizeof(ctx->connector->eld), len));
}
- dev_dbg(dev, "audio copy eld\n");
- memcpy(buf, ctx->connector->eld,
- min(sizeof(ctx->connector->eld), len));
-
return 0;
}
--
2.35.1.1178.g4f1659d476-goog
RE: [Intel-gfx] commit 15512021eb3975a8c2366e3883337e252bb0eee5 causes white spots in console screens
> -Original Message-
> From: Intel-gfx On Behalf Of Jani
> Nikula
> Sent: torstai 14. huhtikuuta 2022 11.32
> To: François Valenduc ; Lisovskiy, Stanislav
>
> Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Subject: Re: [Intel-gfx] commit 15512021eb3975a8c2366e3883337e252bb0eee5
> causes white spots in console screens
>
> On Thu, 14 Apr 2022, François Valenduc wrote:
> > Le 14/04/22 à 10:03, Lisovskiy, Stanislav a écrit :
> >> On Thu, Apr 14, 2022 at 08:33:35AM +0200, François Valenduc wrote:
> >>> Le 14/04/22 à 08:31, Lisovskiy, Stanislav a écrit :
> On Wed, Apr 13, 2022 at 08:12:20PM +0300, Jani Nikula wrote:
> > On Wed, 13 Apr 2022, François Valenduc
> wrote:
> >> Commit 15512021eb3975a8c2366e3883337e252bb0eee5
> >> (15512021eb3975a8c2366e3883337e252bb0eee5) causes a lof of white
> >> spots to appears on the right upper corner of all console screens
> >> (see
> https://drive.google.com/file/d/13GabEvOIKSAj5yox6ybAZEDu3Ixncw5Q/view).
> >> git-bisect shows that this is the offending commit and if I
> >> revert it, the problem goes away. The problem still occurs with
> >> kernel 5.18-rc2 and to all stable trees where it was applied.
> >> Can somebody explains what happens ?
> >>
> >> The video card is the following: VGA compatible controller: Intel
> >> Corporation WhiskeyLake-U GT2 [UHD Graphics 620] (rev 02)
> >> (prog-if 00 [VGA controller])
> >>
> >> Please tell me if you need more info.
> > That's commit 15512021eb39 ("drm/i915: Workaround broken BIOS DBUF
> > configuration on TGL/RKL"), adding Cc's.
> >
> > Please file a report at fdo gitlab [1] and attach dmesg etc. there.
> That commit looks like it is just disabling all the planes, if
> wrong dbuf/wm configuration is detected.
> However it should do that only once during boot as I understand.
>
> Are you sure that is exactly this commit which is causing this?
> Does the issue appear always?
>
> Ville Syrjälä, thoughts?
>
> Stan
>
> > Thanks,
> > Jani.
> >
> >
> > [1]
> > https://gitlab.freedesktop.org/drm/intel/wikis/How-to-file-i915-bu
> > gs
> >
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
> >>> As I said, git-bisect shows it's the offending commit and if I
> >>> revert it, the problem doesn't happens. Otherwise, it always occurs.
> >>>
> >>> François
> >>>
> >> Does it just happen all the time or some steps/certain circumstances
> >> needed for it to happen?
> >>
> >> Only suspicion after looking briefly is that once suspend/resume is
> >> done it might be messing something up. Just a quick guess..
> >>
> >> Stan
> >
> > It occurs permantently as soon as I boot my computer.
>
> I'd appreciate that bug being filed, and the discussion moved there, so all
> the
> details and history are in one place.
Agree.
>
> Thanks,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
Re: [RFC] drm/kms: control display brightness through drm_connector properties
On Mon, 11 Apr 2022, Alex Deucher wrote: > On Mon, Apr 11, 2022 at 6:18 AM Hans de Goede wrote: >> >> Hi, >> >> On 4/8/22 17:11, Alex Deucher wrote: >> > On Fri, Apr 8, 2022 at 10:56 AM Hans de Goede wrote: >> >> >> >> Hi, >> >> >> >> On 4/8/22 16:08, Alex Deucher wrote: >> >>> On Fri, Apr 8, 2022 at 4:07 AM Daniel Vetter wrote: >> >> On Thu, Apr 07, 2022 at 05:05:52PM -0400, Alex Deucher wrote: >> > On Thu, Apr 7, 2022 at 1:43 PM Hans de Goede >> > wrote: >> >> >> >> Hi Simon, >> >> >> >> On 4/7/22 18:51, Simon Ser wrote: >> >>> Very nice plan! Big +1 for the overall approach. >> >> >> >> Thanks. >> >> >> >>> On Thursday, April 7th, 2022 at 17:38, Hans de Goede >> >>> wrote: >> >>> >> The drm_connector brightness properties >> === >> >> bl_brightness: rw 0-int32_max property controlling the brightness >> setting >> of the connected display. The actual maximum of this will be less >> then >> int32_max and is given in bl_brightness_max. >> >>> >> >>> Do we need to split this up into two props for sw/hw state? The >> >>> privacy screen >> >>> stuff needed this, but you're pretty familiar with that. :) >> >> >> >> Luckily that won't be necessary, since the privacy-screen is a >> >> security >> >> feature the firmware/embedded-controller may refuse our requests >> >> (may temporarily lock-out changes) and/or may make changes without >> >> us requesting them itself. Neither is really the case with the >> >> brightness setting of displays. >> >> >> bl_brightness_max: ro 0-int32_max property giving the actual maximum >> of the display's brightness setting. This will report 0 when >> brightness >> control is not available (yet). >> >>> >> >>> I don't think we actually need that one. Integer KMS props all have a >> >>> range which can be fetched via drmModeGetProperty. The max can be >> >>> exposed via this range. Example with the existing alpha prop: >> >>> >> >>> "alpha": range [0, UINT16_MAX] = 65535 >> >> >> >> Right, I already knew that, which is why I explicitly added a range >> >> to the props already. The problem is that the range must be set >> >> before registering the connector and when the backlight driver >> >> only shows up (much) later during boot then we don't know the >> >> range when registering the connector. I guess we could "patch-up" >> >> the range later. But AFAIK that would be a bit of abuse of the >> >> property API as the range is intended to never change, not >> >> even after hotplug uevents. At least atm there is no infra >> >> in the kernel to change the range later. >> >> >> >> Which is why I added an explicit bl_brightness_max property >> >> of which the value gives the actual effective maximum of the >> >> brightness. >> >> Uh ... I'm not a huge fan tbh. The thing is, if we allow hotplugging >> brightness control later on then we just perpetuate the nonsense we have >> right now, forever. >> >> Imo we should support two kinds of drivers: >> >> - drivers which are non-crap, and make sure their backlight driver is >> loaded before they register the drm_device (or at least the >> drm_connector). For those we want the drm_connector->backlight pointer >> to bit static over the lifetime of the connector, and then we can also >> set up the brightness range correctly. >> >> - funny drivers which implement the glorious fallback dance which >> libbacklight implements currently in userspace. Imo for these drivers >> we >> should have a libbacklight_heuristics_backlight, which normalizes or >> whatever, and is also ways there. And then internally handles the >> fallback mess to the "right" backlight driver. >> >> We might have some gaps on acpi systems to make sure the drm driver can >> wait for the backlight driver to show up, but that's about it. >> >> Hotplugging random pieces later on is really not how drivers work >> nowadays >> with deferred probe and component framework and all that. >> >> >> I did consider using the range for this and updating it >> >> on the fly I think nothing is really preventing us from >> >> doing so, but it very much feels like abusing the generic >> >> properties API. >> >> >> bl_brightness_0_is_min_brightness: ro, boolean >> When this is set to true then it is safe to set brightness to 0 >> without worrying that this completely turns the backlight off >> causing >> the screen to become unreadable. When this is false setting >> brightness >> to 0 may turn the backlight off, but this is
Re: [PATCH v3 1/2] drm/dp: Factor out a function to probe a DPCD address
On Mon, 11 Apr 2022, Imre Deak wrote:
> Factor out from drm_dp_dpcd_read() a function to probe a DPCD address
> with a 1-byte read access. This will be needed by the next patch doing a
> read from an LTTPR address, which must happen without the preceding
> wake-up read in drm_dp_dpcd_read().
>
> While at it add tracing for the 1 byte read even if the read was
> successful.
Let's see if that's going to be annoying and confusing...
>
> v2: Add a probe function instead of exporting drm_dp_dpcd_access(). (Jani)
> v3: Add tracing for the 1-byte read even if the read was successful. (Khaled)
>
> Cc: Jani Nikula
> Cc: Khaled Almahallawy
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/dp/drm_dp.c| 33 -
> include/drm/dp/drm_dp_helper.h | 1 +
> 2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/dp/drm_dp.c b/drivers/gpu/drm/dp/drm_dp.c
> index 580016a1b9eb7..2a1f5ff6633cc 100644
> --- a/drivers/gpu/drm/dp/drm_dp.c
> +++ b/drivers/gpu/drm/dp/drm_dp.c
> @@ -527,6 +527,31 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8
> request,
> return ret;
> }
>
> +/**
> + * drm_dp_dpcd_probe() - probe a given DPCD address with a 1-byte read access
> + * @aux: DisplayPort AUX channel (SST)
> + * @offset: address of the register to probe
> + *
> + * Probe the provided DPCD address by reading 1 byte from it. The function
> can
> + * be used to trigger some side-effect the read access has, like waking up
> the
> + * sink, without the need for the read-out value.
> + *
> + * Returns 0 if the read access suceeded, or a negative error code on
> failure.
> + */
> +int drm_dp_dpcd_probe(struct drm_dp_aux *aux, unsigned int offset)
> +{
> + u8 buffer;
> + int ret;
> +
> + ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, &buffer, 1);
> + WARN_ON(ret == 0);
> +
> + drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, &buffer, ret);
> +
> + return ret < 0 ? ret : 0;
> +}
> +EXPORT_SYMBOL(drm_dp_dpcd_probe);
> +
> /**
> * drm_dp_dpcd_read() - read a series of bytes from the DPCD
> * @aux: DisplayPort AUX channel (SST or MST)
> @@ -559,10 +584,9 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux,
> unsigned int offset,
>* monitor doesn't power down exactly after the throw away read.
>*/
> if (!aux->is_remote) {
> - ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV,
> - buffer, 1);
> - if (ret != 1)
> - goto out;
> + ret = drm_dp_dpcd_probe(aux, DP_DPCD_REV);
> + if (ret < 0)
> + return ret;
> }
>
> if (aux->is_remote)
> @@ -571,7 +595,6 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned
> int offset,
> ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset,
>buffer, size);
>
> -out:
> drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, buffer, ret);
> return ret;
> }
> diff --git a/include/drm/dp/drm_dp_helper.h b/include/drm/dp/drm_dp_helper.h
> index 1eccd97419436..91af98e6617c6 100644
> --- a/include/drm/dp/drm_dp_helper.h
> +++ b/include/drm/dp/drm_dp_helper.h
> @@ -2053,6 +2053,7 @@ struct drm_dp_aux {
> bool is_remote;
> };
>
> +int drm_dp_dpcd_probe(struct drm_dp_aux *aux, unsigned int offset);
> ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
>void *buffer, size_t size);
> ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 5.10 1/1] drm/amdgpu: Ensure the AMDGPU file descriptor is legitimate
On Tue, Apr 12, 2022 at 04:20:57PM +0100, Lee Jones wrote: > [ Upstream commit b40a6ab2cf9213923bf8e821ce7fa7f6a0a26990 ] > > This is a partial cherry-pick of the above upstream commit. > > It ensures the file descriptor passed in by userspace is a valid one. > > Cc: Felix Kuehling > Cc: Alex Deucher > Cc: "Christian König" > Cc: David Airlie > Cc: Daniel Vetter > Cc: amd-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Lee Jones > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) Now queued up, thanks. greg k-h
Re: [PATCH 4.19 1/2] drm/amdgpu: Check if fd really is an amdgpu fd.
On Tue, Apr 12, 2022 at 04:35:28PM +0100, Lee Jones wrote: > From: Bas Nieuwenhuizen > > [ Upstream commit 021830d24ba55a578f602979274965344c8e6284 ] > > Otherwise we interpret the file private data as drm & amdgpu data > while it might not be, possibly allowing one to get memory corruption. > > Cc: Felix Kuehling > Cc: Alex Deucher > Cc: "Christian König" > Cc: David Airlie > Cc: Daniel Vetter > Cc: amd-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Bas Nieuwenhuizen > Reviewed-by: Christian König > Signed-off-by: Alex Deucher > Signed-off-by: Lee Jones > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 16 > drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 10 +++--- > 3 files changed, 25 insertions(+), 3 deletions(-) Both now queued up, thanks. greg k-h
Re: [PATCH v6 1/2] dt-bindings: Add DT schema for Arm Mali Valhall GPU
Il 14/04/22 04:50, Nick Fan ha scritto: Add devicetree schema for Arm Mali Valhall GPU Define a compatible string for the Mali Valhall GPU for MediaTek's SoC platform. Signed-off-by: Nick Fan Hello Nick, Unfortunately, this binding is completely wrong. First of all, there's no arm,mali-valhall driver upstream - this will be managed by panfrost later, yes, but right now there's no support. Then, you're also setting opp-microvolt in a way that will never (or, at least, not anytime soon) be supported by the upstream driver, as it manages only one supply for devfreq scaling. Besides, please don't push bindings that have no upstream driver, especially if these are for downstream drivers requiring proprietary components, while a completely open source implementation is in the works. Regards, Angelo
[PATCH v7 0/4] Add support for the eDP panel over aux_bus
This series adds support for generic eDP panel over aux_bus. These changes are dependent on the following series: https://patchwork.kernel.org/project/linux-arm-msm/list/?series=613654&state=* Sankeerth Billakanti (4): drm/msm/dp: Add eDP support via aux_bus drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP drm/msm/dp: wait for hpd high before aux transaction Support the eDP modes given by panel drivers/gpu/drm/msm/dp/dp_aux.c | 21 +++- drivers/gpu/drm/msm/dp/dp_aux.h | 3 +- drivers/gpu/drm/msm/dp/dp_catalog.c | 22 +--- drivers/gpu/drm/msm/dp/dp_catalog.h | 1 + drivers/gpu/drm/msm/dp/dp_display.c | 100 +--- drivers/gpu/drm/msm/dp/dp_display.h | 1 + drivers/gpu/drm/msm/dp/dp_drm.c | 10 ++-- drivers/gpu/drm/msm/dp/dp_parser.c | 23 + drivers/gpu/drm/msm/dp/dp_parser.h | 13 - 9 files changed, 154 insertions(+), 40 deletions(-) -- 2.7.4
[PATCH v7 1/4] drm/msm/dp: Add eDP support via aux_bus
This patch adds support for generic eDP sink through aux_bus. The eDP/DP
controller driver should support aux transactions originating from the
panel-edp driver and hence should be initialized and ready.
The panel bridge supporting the panel should be ready before the bridge
connector is initialized. The generic panel probe needs the controller
resources to be enabled to support the aux transactions originating from
the panel probe.
Signed-off-by: Sankeerth Billakanti
---
Changes in v7:
- aux_bus is mandatory for eDP
- connector type check modified to just check for eDP
Changes in v6:
- Remove initialization
- Fix aux_bus node leak
- Split the patches
drivers/gpu/drm/msm/dp/dp_display.c | 68 ++---
drivers/gpu/drm/msm/dp/dp_display.h | 1 +
drivers/gpu/drm/msm/dp/dp_drm.c | 10 +++---
drivers/gpu/drm/msm/dp/dp_parser.c | 23 ++---
drivers/gpu/drm/msm/dp/dp_parser.h | 13 ++-
5 files changed, 85 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index d7a19d6..43c59cb 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include "msm_drv.h"
#include "msm_kms.h"
@@ -259,14 +260,12 @@ static int dp_display_bind(struct device *dev, struct
device *master,
dp->dp_display.drm_dev = drm;
priv->dp[dp->id] = &dp->dp_display;
- rc = dp->parser->parse(dp->parser, dp->dp_display.connector_type);
+ rc = dp->parser->parse(dp->parser);
if (rc) {
DRM_ERROR("device tree parsing failed\n");
goto end;
}
- dp->dp_display.next_bridge = dp->parser->next_bridge;
-
dp->aux->drm_dev = drm;
rc = dp_aux_register(dp->aux);
if (rc) {
@@ -1319,6 +1318,8 @@ static int dp_display_probe(struct platform_device *pdev)
dp->pdev = pdev;
dp->name = "drm_dp";
dp->dp_display.connector_type = desc->connector_type;
+ dp->dp_display.is_edp =
+ (dp->dp_display.connector_type == DRM_MODE_CONNECTOR_eDP);
rc = dp_init_sub_modules(dp);
if (rc) {
@@ -1508,7 +1509,8 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
dp_hpd_event_setup(dp);
- dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
+ if (!dp_display->is_edp)
+ dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
}
void msm_dp_debugfs_init(struct msm_dp *dp_display, struct drm_minor *minor)
@@ -1530,6 +1532,60 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display,
struct drm_minor *minor)
}
}
+static int dp_display_get_next_bridge(struct msm_dp *dp)
+{
+ int rc;
+ struct dp_display_private *dp_priv;
+ struct device_node *aux_bus;
+ struct device *dev;
+
+ dp_priv = container_of(dp, struct dp_display_private, dp_display);
+ dev = &dp_priv->pdev->dev;
+ aux_bus = of_get_child_by_name(dev->of_node, "aux-bus");
+
+ if (aux_bus && dp->is_edp) {
+ dp_display_host_init(dp_priv);
+ dp_catalog_ctrl_hpd_config(dp_priv->catalog);
+ dp_display_host_phy_init(dp_priv);
+ enable_irq(dp_priv->irq);
+
+ rc = devm_of_dp_aux_populate_ep_devices(dp_priv->aux);
+ of_node_put(aux_bus);
+ if (rc) {
+ disable_irq(dp_priv->irq);
+ dp_display_host_phy_exit(dp_priv);
+ dp_display_host_deinit(dp_priv);
+ return rc;
+ }
+ } else if (dp->is_edp) {
+ DRM_ERROR("eDP aux_bus not found\n");
+ return -ENODEV;
+ }
+
+ /*
+* External bridges are mandatory for eDP interfaces: one has to
+* provide at least an eDP panel (which gets wrapped into panel-bridge).
+*
+* For DisplayPort interfaces external bridges are optional, so
+* silently ignore an error if one is not present (-ENODEV).
+*/
+ rc = dp_parser_find_next_bridge(dp_priv->parser);
+ if (rc == -ENODEV) {
+ if (dp->is_edp) {
+ DRM_ERROR("eDP: next bridge is not present\n");
+ return rc;
+ }
+ } else if (rc) {
+ if (rc != -EPROBE_DEFER)
+ DRM_ERROR("DP: error parsing next bridge: %d\n", rc);
+ return rc;
+ }
+
+ dp->next_bridge = dp_priv->parser->next_bridge;
+
+ return 0;
+}
+
int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
struct drm_encoder *encoder)
{
@@ -1553,6 +1609,10 @@ int msm_dp_modeset_init(struct msm_dp *dp_display,
struct drm_device *dev,
dp_display->encoder = encoder;
+ ret = dp_display_get_next_bridge(dp_display);
+ if (ret)
+ return
[PATCH v7 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP
The panel-edp enables the eDP panel power during probe, get_modes
and enable. The eDP connect and disconnect interrupts for the eDP/DP
controller are directly dependent on panel power. As eDP display can be
assumed as always connected, the controller driver can skip the eDP
connect and disconnect interrupts. Any disruption in the link status
will be indicated via the IRQ_HPD interrupts.
So, the eDP controller driver can just enable the IRQ_HPD and replug
interrupts. The DP controller driver still needs to enable all the
interrupts.
Signed-off-by: Sankeerth Billakanti
---
Changes in v7:
- reordered the patch in the series
- modified the return statement for isr
- connector check modified to just check for eDP
drivers/gpu/drm/msm/dp/dp_catalog.c | 9 +++--
drivers/gpu/drm/msm/dp/dp_display.c | 22 +-
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index fac815f..07f2389 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -569,10 +569,6 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog
*dp_catalog)
u32 reftimer = dp_read_aux(catalog, REG_DP_DP_HPD_REFTIMER);
- /* enable HPD plug and unplug interrupts */
- dp_catalog_hpd_config_intr(dp_catalog,
- DP_DP_HPD_PLUG_INT_MASK | DP_DP_HPD_UNPLUG_INT_MASK, true);
-
/* Configure REFTIMER and enable it */
reftimer |= DP_DP_HPD_REFTIMER_ENABLE;
dp_write_aux(catalog, REG_DP_DP_HPD_REFTIMER, reftimer);
@@ -599,13 +595,14 @@ u32 dp_catalog_hpd_get_intr_status(struct dp_catalog
*dp_catalog)
{
struct dp_catalog_private *catalog = container_of(dp_catalog,
struct dp_catalog_private, dp_catalog);
- int isr = 0;
+ int isr, mask;
isr = dp_read_aux(catalog, REG_DP_DP_HPD_INT_STATUS);
dp_write_aux(catalog, REG_DP_DP_HPD_INT_ACK,
(isr & DP_DP_HPD_INT_MASK));
+ mask = dp_read_aux(catalog, REG_DP_DP_HPD_INT_MASK);
- return isr;
+ return isr & (mask | ~DP_DP_HPD_INT_MASK);
}
int dp_catalog_ctrl_get_interrupt(struct dp_catalog *dp_catalog)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 43c59cb..1339c45 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -683,7 +683,8 @@ static int dp_hpd_unplug_handle(struct dp_display_private
*dp, u32 data)
dp_display_handle_plugged_change(&dp->dp_display, false);
/* enable HDP plug interrupt to prepare for next plugin */
- dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK, true);
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
DP_DP_HPD_PLUG_INT_MASK, true);
DRM_DEBUG_DP("After, type=%d hpd_state=%d\n",
dp->dp_display.connector_type, state);
@@ -1096,6 +1097,13 @@ static void dp_display_config_hpd(struct
dp_display_private *dp)
dp_display_host_init(dp);
dp_catalog_ctrl_hpd_config(dp->catalog);
+ /* Enable plug and unplug interrupts only for external DisplayPort */
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
+ DP_DP_HPD_PLUG_INT_MASK |
+ DP_DP_HPD_UNPLUG_INT_MASK,
+ true);
+
/* Enable interrupt first time
* we are leaving dp clocks on during disconnect
* and never disable interrupt
@@ -1381,6 +1389,12 @@ static int dp_pm_resume(struct device *dev)
dp_catalog_ctrl_hpd_config(dp->catalog);
+ if (!dp->dp_display.is_edp)
+ dp_catalog_hpd_config_intr(dp->catalog,
+ DP_DP_HPD_PLUG_INT_MASK |
+ DP_DP_HPD_UNPLUG_INT_MASK,
+ true);
+
if (dp_catalog_link_is_connected(dp->catalog)) {
/*
* set sink to normal operation mode -- D0
@@ -1654,6 +1668,9 @@ void dp_bridge_enable(struct drm_bridge *drm_bridge)
return;
}
+ if (dp->is_edp)
+ dp_hpd_plug_handle(dp_display, 0);
+
mutex_lock(&dp_display->event_mutex);
/* stop sentinel checking */
@@ -1718,6 +1735,9 @@ void dp_bridge_post_disable(struct drm_bridge *drm_bridge)
dp_display = container_of(dp, struct dp_display_private, dp_display);
+ if (dp->is_edp)
+ dp_hpd_unplug_handle(dp_display, 0);
+
mutex_lock(&dp_display->event_mutex);
/* stop sentinel checking */
--
2.7.4
[PATCH v7 3/4] drm/msm/dp: wait for hpd high before aux transaction
The source device should ensure the sink is ready before proceeding to
read the sink capability or perform any aux transactions. The sink
will indicate its readiness by asserting the HPD line. The controller
driver needs to wait for the hpd line to be asserted by the sink before
it performs any aux transactions.
The eDP sink is assumed to be always connected. It needs power from the
source and its HPD line will be asserted only after the panel is powered
on. The panel power will be enabled from the panel-edp driver and only
after that, the hpd line will be asserted.
Whereas for DP, the sink can be hotplugged and unplugged anytime. The hpd
line gets asserted to indicate the sink is connected and ready. Hence
there is no need to wait for the hpd line to be asserted for a DP sink.
Signed-off-by: Sankeerth Billakanti
---
Changes in v7:
- add a comment to say why the wait si done for eDP
- correct the commit text
Changes in v6:
- Wait for hpd high only for eDP
- Split into smaller patches
drivers/gpu/drm/msm/dp/dp_aux.c | 21 -
drivers/gpu/drm/msm/dp/dp_aux.h | 3 ++-
drivers/gpu/drm/msm/dp/dp_catalog.c | 13 +
drivers/gpu/drm/msm/dp/dp_catalog.h | 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
5 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
index 6d36f63..cf0739f 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -36,6 +36,7 @@ struct dp_aux_private {
bool initted;
u32 offset;
u32 segment;
+ bool is_edp;
struct drm_dp_aux dp_aux;
};
@@ -337,6 +338,22 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
goto exit;
}
+ /*
+* For eDP it's important to give a reasonably long wait here for HPD
+* to be asserted. This is because the panel driver may have _just_
+* turned on the panel and then tried to do an AUX transfer. The panel
+* driver has no way of knowing when the panel is ready, so it's up
+* to us to wait. For DP we never get into this situation so let's
+* avoid ever doing the extra long wait for DP.
+*/
+ if (aux->is_edp) {
+ ret = dp_catalog_aux_wait_for_hpd_connect_state(aux->catalog);
+ if (ret) {
+ DRM_DEBUG_DP("Panel not ready for aux transactions\n");
+ goto exit;
+ }
+ }
+
dp_aux_update_offset_and_segment(aux, msg);
dp_aux_transfer_helper(aux, msg, true);
@@ -491,7 +508,8 @@ void dp_aux_unregister(struct drm_dp_aux *dp_aux)
drm_dp_aux_unregister(dp_aux);
}
-struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog)
+struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog,
+ bool is_edp)
{
struct dp_aux_private *aux;
@@ -506,6 +524,7 @@ struct drm_dp_aux *dp_aux_get(struct device *dev, struct
dp_catalog *catalog)
init_completion(&aux->comp);
aux->cmd_busy = false;
+ aux->is_edp = is_edp;
mutex_init(&aux->mutex);
aux->dev = dev;
diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h
index 82afc8d..c99aeec 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.h
+++ b/drivers/gpu/drm/msm/dp/dp_aux.h
@@ -16,7 +16,8 @@ void dp_aux_init(struct drm_dp_aux *dp_aux);
void dp_aux_deinit(struct drm_dp_aux *dp_aux);
void dp_aux_reconfig(struct drm_dp_aux *dp_aux);
-struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog);
+struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog,
+ bool is_edp);
void dp_aux_put(struct drm_dp_aux *aux);
#endif /*__DP_AUX_H_*/
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 07f2389..4282a98 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -242,6 +242,19 @@ void dp_catalog_aux_update_cfg(struct dp_catalog
*dp_catalog)
phy_calibrate(phy);
}
+int dp_catalog_aux_wait_for_hpd_connect_state(struct dp_catalog *dp_catalog)
+{
+ u32 state;
+ struct dp_catalog_private *catalog = container_of(dp_catalog,
+ struct dp_catalog_private, dp_catalog);
+
+ /* poll for hpd connected status every 2ms and timeout after 500ms */
+ return readl_poll_timeout(catalog->io->dp_controller.aux.base +
+ REG_DP_DP_HPD_INT_STATUS,
+ state, state & DP_DP_HPD_STATE_STATUS_CONNECTED,
+ 2000, 50);
+}
+
static void dump_regs(void __iomem *base, int len)
{
int i;
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 7dea101..45140a3 100644
--- a/drivers/gpu/drm/msm/dp/dp_catal
[PATCH v7 4/4] Support the eDP modes given by panel
The eDP controller does not have a reliable way keep panel powered on to read the sink capabilities. So, the controller driver cannot validate if a mode can be supported by the source. We will rely on the panel driver to populate only the supported modes for now. Signed-off-by: Sankeerth Billakanti --- drivers/gpu/drm/msm/dp/dp_display.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index c7277f0..0f18a16 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -998,6 +998,14 @@ enum drm_mode_status dp_bridge_mode_valid(struct drm_bridge *bridge, return -EINVAL; } + /* +* The eDP controller currently does not have a reliable way of +* enabling panel power to read sink capabilities. So, we rely +* on the panel driver to populate only supported modes for now. +*/ + if (dp->is_edp) + return MODE_OK; + if ((dp->max_pclk_khz <= 0) || (dp->max_pclk_khz > DP_MAX_PIXEL_CLK_KHZ) || (mode->clock > dp->max_pclk_khz)) -- 2.7.4
Re: refactor the i915 GVT support and move to the modern mdev API v3
On 4/13/22 11:20 PM, Jason Gunthorpe wrote: > On Wed, Apr 13, 2022 at 11:13:06PM +, Wang, Zhi A wrote: >> Hi folks: >> >> Thanks so much for the efforts. I prepared a branch which contains all our >> patches.The aim of the branch is for the VFIO maintainers to pull the whole >> bunch easily after the drm-intel-next got merged through drm (as one of the >> MMIO patches depends on a patch in drm-intel-next). >> >> I dropped patch 4 and patch 5 as they have been covered by Jani's patches. >> Some conflicts was solved. >> QA is going to test it today. >> >> You can find it here: >> >> git clone https://github.com/intel/gvt-linux -b for-christoph > > There are alot of extra commits on there - is it possible to base this > straight on rc1 not on some kind of existing DRM tree? > > Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly > from frontbuffer flush as a base? > > Jason > Hi Jason: I updated the branch. You can check if those are what you are expecting. :) Thanks, Zhi.
[PATCH 0/1] add support for enum module parameters
Hey, I've sent this before, ages ago, but haven't really followed through with it. I still think it would be useful for many scenarios where a plain number is a clumsy interface for a module param. Thoughts? BR, Jani. Cc: Andrew Morton Cc: Greg Kroah-Hartman Cc: Lucas De Marchi Jani Nikula (1): module: add enum module parameter type to map names to values include/linux/moduleparam.h | 64 + kernel/params.c | 41 2 files changed, 105 insertions(+) -- 2.30.2
[PATCH 1/1] module: add enum module parameter type to map names to values
Add enum module parameter type that's internally an int and externally
maps names to values. This makes the userspace interface more intuitive
to use and somewhat easier to document, while also limiting the allowed
values set by userspace to the defined set.
For example, given this code to define a "mode" in a fictional module
"foobar":
struct module_param_enumeration modes[] = {
{ "foo", 0 },
{ "bar", 1 },
{ "baz", -1 },
};
int mode;
module_param_enum(mode, modes, 0600);
You can probe foobar with "foobar.mode=bar" in the kernel or modprobe
command line to set the mode to 1.
Similarly, you can use the sysfs with the names:
# echo baz > /sys/module/foobar/parameters/mode
# cat /sys/module/foobar/parameters/mode
baz
With checks:
# echo nope > /sys/module/foobar/parameters/mode
echo: write error: Invalid argument
Of course, the kernel can still internally set the mode variable
directly to a value that is not defined in the enumerations (obviously
to be avoided), which will result in unknown key:
# cat /sys/module/foobar/parameters/mode
(unknown)
Cc: Andrew Morton
Cc: Greg Kroah-Hartman
Cc: Lucas De Marchi
Signed-off-by: Jani Nikula
---
include/linux/moduleparam.h | 64 +
kernel/params.c | 41
2 files changed, 105 insertions(+)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 962cd41a2cb5..a11fb8f214e5 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -77,6 +77,7 @@ struct kernel_param {
void *arg;
const struct kparam_string *str;
const struct kparam_array *arr;
+ const struct kparam_enum *enumeration;
};
};
@@ -98,6 +99,19 @@ struct kparam_array
void *elem;
};
+/* Special ones for enums */
+struct module_param_enumeration {
+ const char *key;
+ int val;
+};
+
+struct kparam_enum
+{
+ const struct module_param_enumeration *enums;
+ unsigned int num_enums;
+ int *val;
+};
+
/**
* module_param - typesafe helper for a module/cmdline parameter
* @name: the variable to alter, and exposed parameter name.
@@ -484,6 +498,11 @@ extern int param_set_bint(const char *val, const struct
kernel_param *kp);
#define param_get_bint param_get_int
#define param_check_bint param_check_int
+/* An enumeration */
+extern const struct kernel_param_ops param_ops_enum;
+extern int param_set_enum(const char *key, const struct kernel_param *kp);
+extern int param_get_enum(char *buffer, const struct kernel_param *kp);
+
/**
* module_param_array - a parameter which is an array of some type
* @name: the name of the array variable
@@ -523,6 +542,51 @@ extern int param_set_bint(const char *val, const struct
kernel_param *kp);
perm, -1, 0); \
__MODULE_PARM_TYPE(name, "array of " #type)
+/**
+ * module_param_enum - a parameter which is an enumeration
+ * @name: the variable to alter, and exposed parameter name
+ * @enumerations: array of struct module_param_enumeration defining the enums
+ * @perm: visibility in sysfs
+ *
+ * The userspace input and output are based on the names defined in the
+ * @enumerations array, which maps the names to values stored in the int
+ * variable defined by @name.
+ *
+ * When initializing or changing the variable @name, ensure the value is
defined
+ * in @enumerations. Otherwise, reading the parameter value via sysfs will
+ * output "(unknown)".
+ *
+ * ARRAY_SIZE(@enumerations) is used to determine the number of elements in the
+ * enumerations array, so the definition must be visible.
+ */
+
+#define module_param_enum(name, enumerations, perm)\
+ module_param_enum_named(name, name, enumerations, perm)
+
+/**
+ * module_param_enum_named - a renamed parameter which is an enumeration
+ * @name: a valid C identifier which is the parameter name
+ * @value: the actual lvalue int variable to alter
+ * @enumerations: array of struct module_param_enumeration defining the enums
+ * @perm: visibility in sysfs
+ *
+ * This exposes a different name than the actual variable name. See
+ * module_param_named() for why this might be necessary.
+ */
+#define module_param_enum_named(name, value, enumerations, perm) \
+ param_check_int(name, &(value));\
+ static const struct kparam_enum __param_arr_##name =\
+ { \
+ .enums = enumerations, \
+ .num_enums = ARRAY_SIZE(enumerations), \
+ .val = &value \
+ };
Re: [PATCH v6 1/2] dt-bindings: Add DT schema for Arm Mali Valhall GPU
On 14/04/2022 12:51, AngeloGioacchino Del Regno wrote: > Il 14/04/22 04:50, Nick Fan ha scritto: >> Add devicetree schema for Arm Mali Valhall GPU >> >> Define a compatible string for the Mali Valhall GPU >> for MediaTek's SoC platform. >> >> Signed-off-by: Nick Fan > > Hello Nick, > Unfortunately, this binding is completely wrong. I think that's unfair, although there is room for improvement. > First of all, there's no arm,mali-valhall driver upstream - this will be > managed > by panfrost later, yes, but right now there's no support. We need a binding agreed upon before support can be added. > Then, you're also setting opp-microvolt in a way that will never (or, at > least, > not anytime soon) be supported by the upstream driver, as it manages > only one > supply for devfreq scaling. The mt8183 binding (already in tree) is very similar. The binding also should be describing the hardware not what the driver supports. There are indeed limitations in Panfrost for supporting multiple supplies, but that's something that needs improving in the driver not a reason to block a (presumably correct) description of the hardware. I can't comments on whether the specifics of the mt8192 are correct. > Besides, please don't push bindings that have no upstream driver, > especially if > these are for downstream drivers requiring proprietary components, while a > completely open source implementation is in the works. More constructively, Alyssa has already posted a patch (as part of the series adding driver support) which would extend the existing Bifrost bindings to (pre-CSF) Valhall: https://lore.kernel.org/dri-devel/20220211202728.6146-2-alyssa.rosenzw...@collabora.com/ I'm not sure I see the point of having a separate binding document for Valhall considering the (pre-CSF) hardware is the same from the kernel perspective. Thanks, Steve
Re: [PATCH v4 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings
On Wed, 13 Apr 2022 18:23:54 +0200, Javier Martinez Canillas wrote: > The current compatible strings for SSD130x I2C controllers contain both an > "fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver > and also that are for devices that can be accessed over an I2C bus. > > But a DT is supposed to describe the hardware and not Linux implementation > details. So let's deprecate those compatible strings and add new ones that > only contain the vendor and device name, without any of these suffixes. > > These will just describe the device and can be matched by both I2C and SPI > DRM drivers. The required properties should still be enforced for old ones. > > While being there, just drop the "sinowealth,sh1106-i2c" compatible string > since that was never present in a released Linux version. > > Signed-off-by: Javier Martinez Canillas > Acked-by: Mark Brown > Reviewed-by: Geert Uytterhoeven > --- > > (no changes since v3) > > Changes in v3: > - Drop the "sinowealth,sh1106-i2c", wasn't in a released version (Chen-Yu > Tsai) > - Continue enforcing required properties for deprecated strings (Maxime > Ripard) > > Changes in v2: > - Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven) > > .../bindings/display/solomon,ssd1307fb.yaml | 44 +-- > 1 file changed, 31 insertions(+), 13 deletions(-) > Reviewed-by: Rob Herring
Re: [PATCH v4 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers
On Wed, 13 Apr 2022 18:23:55 +0200, Javier Martinez Canillas wrote: > The Solomon SSD130x OLED displays can either have an I2C or SPI interface, > add to the schema the properties and examples for OLED devices under SPI. > > Signed-off-by: Javier Martinez Canillas > Acked-by: Mark Brown > Reviewed-by: Geert Uytterhoeven > --- > > Changes in v4: > - Add a description for the dc-gpios property for SPI (Geert Uytterhoeven) > > Changes in v3: > - Add a comment to the properties required for SPI (Geert Uytterhoeven) > > Changes in v2: > - Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven) > > .../bindings/display/solomon,ssd1307fb.yaml | 42 ++- > 1 file changed, 40 insertions(+), 2 deletions(-) > Reviewed-by: Rob Herring
Re: [RFC] drm/kms: control display brightness through drm_connector properties
On Thu, 07 Apr 2022, Hans de Goede wrote: > As discussed already several times in the past: > https://www.x.org/wiki/Events/XDC2014/XDC2014GoedeBacklight/ > > https://lore.kernel.org/all/4b17ba08-39f3-57dd-5aad-d37d844b0...@linux.intel.com/ > > The current userspace API for brightness control offered by > /sys/class/backlight devices has various issues, the biggest 2 being: > > 1. There is no way to map the backlight device to a specific >display-output / panel (1) > 2. Controlling the brightness requires root-rights requiring >desktop-environments to use suid-root helpers for this. > > As already discussed on various conference's hallway tracks > and as has been proposed on the dri-devel list once before (2), > it seems that there is consensus that the best way to to solve these > 2 issues is to add support for controlling a video-output's brightness > through properties on the drm_connector. > > This RFC outlines my plan to try and actually implement this, > which has 3 phases: > > > Phase 1: Stop registering multiple /sys/class/backlight devs for a single > display > = > > On x86 there can be multiple firmware + direct-hw-access methods > for controlling the backlight and in some cases the kernel registers > multiple backlight-devices for a single internal laptop LCD panel: > > a) i915 and nouveau unconditionally register their "native" backlight dev >even on devices where /sys/class/backlight/acpi_video0 must be used >to control the backlight, relying on userspace to prefer the "firmware" >acpi_video0 device over "native" devices. > b) amdgpu and nouveau rely on the acpi_video driver initializing before >them, which currently causes /sys/class/backlight/acpi_video0 to usually >show up and then they register their own native backlight driver after >which the drivers/acpi/video_detect.c code unregisters the acpi_video0 >device. This means that userspace briefly sees 2 devices and the >disappearing of acpi_video0 after a brief time confuses the systemd >backlight level save/restore code, see e.g.: >https://bbs.archlinux.org/viewtopic.php?id=269920 > > I already have a pretty detailed plan to tackle this, which I will > post in a separate RFC email. I plan to start working on this right > away, as it will be good to have this fixed regardless. > > > Phase 2: Add drm_connector properties mirroring the matching backlight device > = > > The plan is to add a drm_connector helper function, which optionally takes > a pointer to the backlight device for the GPU's native backlight device, > which will then mirror the backlight settings from the backlight device > in a set of read/write brightness* properties on the connector. > > This function can then be called by GPU drivers for the drm_connector for > the internal panel and it will then take care of everything. When there > is no native GPU backlight device, or when it should not be used then > (on x86) the helper will use the acpi_video_get_backlight_type() to > determine which backlight-device should be used instead and it will find > + mirror that one. > > > Phase 3: Deprecate /sys/class/backlight uAPI > > > Once most userspace has moved over to using the new drm_connector > brightness props, a Kconfig option can be added to stop exporting > the backlight-devices under /sys/class/backlight. The plan is to > just disable the sysfs interface and keep the existing backlight-device > internal kernel abstraction as is, since some abstraction for (non GPU > native) backlight devices will be necessary regardless. > > An alternative to disabling the sysfs class entirely, would be > to allow setting it to read-only through Kconfig. > > > What scale to use for the drm_connector bl_brightness property? > === > > The tricky part of this plan is phase 2 and then esp. defining what the > new brightness properties will look like and how they will work. > > The biggest challenge here is to decide on a fixed scale for the main > brightness property, say 0-65535, using scaling where the actual hw scale > is different, or if this should simply be a 1:1 mirror of the current > backlight interface, with the actual hw scale / brightness_max value > exposed as a drm_connector property. > > 1:1 advantages / 0-65535 disadvantages > - Userspace will likely move over to the connector-props quite slowly and > we can expect various userspace bits, esp. also custom user scripts, to > keep using the old uAPI for a long time. Using the 2 APIs are intermixed > is fine when using a 1:1 brightness scale mapping. But if we end up doing > a scaling round-trip all the time then eventually the brightness is going > do drift. This can even happen if the user never changes the bri
Re: [PATCHv4] drm/amdgpu: disable ASPM on Intel Alder Lake based systems
On Thu, Apr 14, 2022 at 3:52 AM Paul Menzel wrote:
>
> [Cc: -kernel test robot ]
>
> Dear Alex, dear Richard,
>
>
> Am 13.04.22 um 15:00 schrieb Alex Deucher:
> > On Wed, Apr 13, 2022 at 3:43 AM Paul Menzel wrote:
>
> >> Thank you for sending out v4.
> >>
> >> Am 12.04.22 um 23:50 schrieb Richard Gong:
> >>> Active State Power Management (ASPM) feature is enabled since kernel 5.14.
> >>> There are some AMD GFX cards (such as WX3200 and RX640) that won't work
> >>> with ASPM-enabled Intel Alder Lake based systems. Using these GFX cards as
> >>> video/display output, Intel Alder Lake based systems will hang during
> >>> suspend/resume.
> >>
> >> I am still not clear, what “hang during suspend/resume” means. I guess
> >> suspending works fine? During resume (S3 or S0ix?), where does it hang?
> >> The system is functional, but there are only display problems?
> >>
> >>> The issue was initially reported on one system (Dell Precision 3660 with
> >>> BIOS version 0.14.81), but was later confirmed to affect at least 4 Alder
> >>> Lake based systems.
> >>>
> >>> Add extra check to disable ASPM on Intel Alder Lake based systems.
> >>>
> >>> Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default")
> >>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1885
> >>> Reported-by: kernel test robot
> >>
> >> This tag is a little confusing. Maybe clarify that it was for an issue
> >> in a previous patch iteration?
> >>
> >>> Signed-off-by: Richard Gong
> >>> ---
> >>> v4: s/CONFIG_X86_64/CONFIG_X86
> >>> enhanced check logic
> >>> v3: s/intel_core_asom_chk/aspm_support_quirk_check
> >>> correct build error with W=1 option
> >>> v2: correct commit description
> >>> move the check from chip family to problematic platform
> >>> ---
> >>>drivers/gpu/drm/amd/amdgpu/vi.c | 17 -
> >>>1 file changed, 16 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
> >>> b/drivers/gpu/drm/amd/amdgpu/vi.c
> >>> index 039b90cdc3bc..b33e0a9bee65 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> >>> @@ -81,6 +81,10 @@
> >>>#include "mxgpu_vi.h"
> >>>#include "amdgpu_dm.h"
> >>>
> >>> +#if IS_ENABLED(CONFIG_X86)
> >>> +#include
> >>> +#endif
> >>> +
> >>>#define ixPCIE_LC_L1_PM_SUBSTATE0x100100C6
> >>>#define PCIE_LC_L1_PM_SUBSTATE__LC_L1_SUBSTATES_OVERRIDE_EN_MASK
> >>> 0x0001L
> >>>#define PCIE_LC_L1_PM_SUBSTATE__LC_PCI_PM_L1_2_OVERRIDE_MASK
> >>> 0x0002L
> >>> @@ -1134,13 +1138,24 @@ static void vi_enable_aspm(struct amdgpu_device
> >>> *adev)
> >>>WREG32_PCIE(ixPCIE_LC_CNTL, data);
> >>>}
> >>>
> >>> +static bool aspm_support_quirk_check(void)
> >>> +{
> >>> + if (IS_ENABLED(CONFIG_X86)) {
> >>> + struct cpuinfo_x86 *c = &cpu_data(0);
> >>> +
> >>> + return !(c->x86 == 6 && c->x86_model ==
> >>> INTEL_FAM6_ALDERLAKE);
> >>> + }
> >>> +
> >>> + return true;
> >>> +}
> >>> +
> >>>static void vi_program_aspm(struct amdgpu_device *adev)
> >>>{
> >>>u32 data, data1, orig;
> >>>bool bL1SS = false;
> >>>bool bClkReqSupport = true;
> >>>
> >>> - if (!amdgpu_device_should_use_aspm(adev))
> >>> + if (!amdgpu_device_should_use_aspm(adev) ||
> >>> !aspm_support_quirk_check())
> >>>return;
> >>
> >> Can users still forcefully enable ASPM with the parameter `amdgpu.aspm`?
> >>
> >>>
> >>>if (adev->flags & AMD_IS_APU ||
> >>
> >> If I remember correctly, there were also newer cards, where ASPM worked
> >> with Intel Alder Lake, right? Can only the problematic generations for
> >> WX3200 and RX640 be excluded from ASPM?
> >
> > This patch only disables it for the generation that was problematic.
>
> Could that please be made clear in the commit message summary, and message?
Sure. Richard, please add that this only disables ASPM on VI parts
when in an alderlake system.
>
> Loosely related, is there a public (or internal issue) to analyze how to
> get ASPM working for VI generation devices with Intel Alder Lake?
We'd need support from Intel. I'm not sure where things currently stand.
Alex
Re: [PATCH v3 1/5] drm/i915: instantiate ttm ranger manager for stolen memory
On 4/12/22 21:38, Robert Beckett wrote:
prepare for ttm based stolen region by using ttm range manager
as the resource manager for stolen region.
Signed-off-by: Robert Beckett
LGTM.
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 6 ++--
drivers/gpu/drm/i915/intel_region_ttm.c | 31 +++-
2 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index a10716f4e717..358f8a1a30ce 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -58,11 +58,13 @@ i915_ttm_region(struct ttm_device *bdev, int ttm_mem_type)
struct drm_i915_private *i915 = container_of(bdev, typeof(*i915), bdev);
/* There's some room for optimization here... */
- GEM_BUG_ON(ttm_mem_type != I915_PL_SYSTEM &&
- ttm_mem_type < I915_PL_LMEM0);
+ GEM_BUG_ON(ttm_mem_type == I915_PL_GGTT);
+
if (ttm_mem_type == I915_PL_SYSTEM)
return intel_memory_region_lookup(i915, INTEL_MEMORY_SYSTEM,
0);
+ if (ttm_mem_type == I915_PL_STOLEN)
+ return i915->mm.stolen_region;
return intel_memory_region_lookup(i915, INTEL_MEMORY_LOCAL,
ttm_mem_type - I915_PL_LMEM0);
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c
b/drivers/gpu/drm/i915/intel_region_ttm.c
index 62ff77445b01..7d49ea72e33f 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -49,7 +49,7 @@ void intel_region_ttm_device_fini(struct drm_i915_private
*dev_priv)
/*
* Map the i915 memory regions to TTM memory types. We use the
- * driver-private types for now, reserving TTM_PL_VRAM for stolen
+ * driver-private types for now, reserving I915_PL_STOLEN for stolen
* memory and TTM_PL_TT for GGTT use if decided to implement this.
*/
int intel_region_to_ttm_type(const struct intel_memory_region *mem)
@@ -58,11 +58,17 @@ int intel_region_to_ttm_type(const struct
intel_memory_region *mem)
GEM_BUG_ON(mem->type != INTEL_MEMORY_LOCAL &&
mem->type != INTEL_MEMORY_MOCK &&
- mem->type != INTEL_MEMORY_SYSTEM);
+ mem->type != INTEL_MEMORY_SYSTEM &&
+ mem->type != INTEL_MEMORY_STOLEN_SYSTEM &&
+ mem->type != INTEL_MEMORY_STOLEN_LOCAL);
if (mem->type == INTEL_MEMORY_SYSTEM)
return TTM_PL_SYSTEM;
+ if (mem->type == INTEL_MEMORY_STOLEN_SYSTEM ||
+ mem->type == INTEL_MEMORY_STOLEN_LOCAL)
+ return I915_PL_STOLEN;
+
type = mem->instance + TTM_PL_PRIV;
GEM_BUG_ON(type >= TTM_NUM_MEM_TYPES);
@@ -86,10 +92,16 @@ int intel_region_ttm_init(struct intel_memory_region *mem)
int mem_type = intel_region_to_ttm_type(mem);
int ret;
- ret = i915_ttm_buddy_man_init(bdev, mem_type, false,
- resource_size(&mem->region),
- mem->io_size,
- mem->min_page_size, PAGE_SIZE);
+ if (mem_type == I915_PL_STOLEN) {
+ ret = ttm_range_man_init(bdev, mem_type, false,
+resource_size(&mem->region) >>
PAGE_SHIFT);
+ mem->is_range_manager = true;
+ } else {
+ ret = i915_ttm_buddy_man_init(bdev, mem_type, false,
+ resource_size(&mem->region),
+ mem->io_size,
+ mem->min_page_size, PAGE_SIZE);
+ }
if (ret)
return ret;
@@ -109,6 +121,7 @@ int intel_region_ttm_init(struct intel_memory_region *mem)
int intel_region_ttm_fini(struct intel_memory_region *mem)
{
struct ttm_resource_manager *man = mem->region_private;
+ int mem_type = intel_region_to_ttm_type(mem);
int ret = -EBUSY;
int count;
@@ -139,8 +152,10 @@ int intel_region_ttm_fini(struct intel_memory_region *mem)
if (ret || !man)
return ret;
- ret = i915_ttm_buddy_man_fini(&mem->i915->bdev,
- intel_region_to_ttm_type(mem));
+ if (mem_type == I915_PL_STOLEN)
+ ret = ttm_range_man_fini(&mem->i915->bdev, mem_type);
+ else
+ ret = i915_ttm_buddy_man_fini(&mem->i915->bdev, mem_type);
GEM_WARN_ON(ret);
mem->region_private = NULL;
Re: [Intel-gfx] commit 15512021eb3975a8c2366e3883337e252bb0eee5 causes white spots in console screens
Le 14/04/22 à 11:51, Saarinen, Jani a écrit :
-Original Message-
From: Intel-gfx On Behalf Of Jani
Nikula
Sent: torstai 14. huhtikuuta 2022 11.32
To: François Valenduc ; Lisovskiy, Stanislav
Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] commit 15512021eb3975a8c2366e3883337e252bb0eee5
causes white spots in console screens
On Thu, 14 Apr 2022, François Valenduc wrote:
Le 14/04/22 à 10:03, Lisovskiy, Stanislav a écrit :
On Thu, Apr 14, 2022 at 08:33:35AM +0200, François Valenduc wrote:
Le 14/04/22 à 08:31, Lisovskiy, Stanislav a écrit :
On Wed, Apr 13, 2022 at 08:12:20PM +0300, Jani Nikula wrote:
On Wed, 13 Apr 2022, François Valenduc
wrote:
Commit 15512021eb3975a8c2366e3883337e252bb0eee5
(15512021eb3975a8c2366e3883337e252bb0eee5) causes a lof of white
spots to appears on the right upper corner of all console screens
(see
https://drive.google.com/file/d/13GabEvOIKSAj5yox6ybAZEDu3Ixncw5Q/view).
git-bisect shows that this is the offending commit and if I
revert it, the problem goes away. The problem still occurs with
kernel 5.18-rc2 and to all stable trees where it was applied.
Can somebody explains what happens ?
The video card is the following: VGA compatible controller: Intel
Corporation WhiskeyLake-U GT2 [UHD Graphics 620] (rev 02)
(prog-if 00 [VGA controller])
Please tell me if you need more info.
That's commit 15512021eb39 ("drm/i915: Workaround broken BIOS DBUF
configuration on TGL/RKL"), adding Cc's.
Please file a report at fdo gitlab [1] and attach dmesg etc. there.
That commit looks like it is just disabling all the planes, if
wrong dbuf/wm configuration is detected.
However it should do that only once during boot as I understand.
Are you sure that is exactly this commit which is causing this?
Does the issue appear always?
Ville Syrjälä, thoughts?
Stan
Thanks,
Jani.
[1]
https://gitlab.freedesktop.org/drm/intel/wikis/How-to-file-i915-bu
gs
--
Jani Nikula, Intel Open Source Graphics Center
As I said, git-bisect shows it's the offending commit and if I
revert it, the problem doesn't happens. Otherwise, it always occurs.
François
Does it just happen all the time or some steps/certain circumstances
needed for it to happen?
Only suspicion after looking briefly is that once suspend/resume is
done it might be messing something up. Just a quick guess..
Stan
It occurs permantently as soon as I boot my computer.
I'd appreciate that bug being filed, and the discussion moved there, so all the
details and history are in one place.
Agree.
Thanks,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
As requested, I filled a bug on freedesktop
(https://gitlab.freedesktop.org/drm/intel/-/issues/5711)
[PATCH 4.19 202/338] video: fbdev: nvidiafb: Use strscpy() to prevent buffer overflow
From: Tim Gardner
[ Upstream commit 37a1a2e6eeeb101285cd34e12e48a881524701aa ]
Coverity complains of a possible buffer overflow. However,
given the 'static' scope of nvidia_setup_i2c_bus() it looks
like that can't happen after examiniing the call sites.
CID 19036 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW)
1. fixed_size_dest: You might overrun the 48-character fixed-size string
chan->adapter.name by copying name without checking the length.
2. parameter_as_source: Note: This defect has an elevated risk because the
source argument is a parameter of the current function.
89strcpy(chan->adapter.name, name);
Fix this warning by using strscpy() which will silence the warning and
prevent any future buffer overflows should the names used to identify the
channel become much longer.
Cc: Antonino Daplas
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Tim Gardner
Signed-off-by: Helge Deller
Signed-off-by: Sasha Levin
---
drivers/video/fbdev/nvidia/nv_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/nvidia/nv_i2c.c
b/drivers/video/fbdev/nvidia/nv_i2c.c
index d7994a173245..0b48965a6420 100644
--- a/drivers/video/fbdev/nvidia/nv_i2c.c
+++ b/drivers/video/fbdev/nvidia/nv_i2c.c
@@ -86,7 +86,7 @@ static int nvidia_setup_i2c_bus(struct nvidia_i2c_chan *chan,
const char *name,
{
int rc;
- strcpy(chan->adapter.name, name);
+ strscpy(chan->adapter.name, name, sizeof(chan->adapter.name));
chan->adapter.owner = THIS_MODULE;
chan->adapter.class = i2c_class;
chan->adapter.algo_data = &chan->algo;
--
2.34.1
Re: [PATCH] drm/radeon: Add build directory to include path
On Thu, Apr 14, 2022 at 4:44 AM Christian König wrote: > > Am 14.04.22 um 09:37 schrieb Michel Dänzer: > > On 2022-04-14 08:24, Christian König wrote: > >> Am 13.04.22 um 18:14 schrieb Michel Dänzer: > >>> From: Michel Dänzer > >>> > >>> Fixes compile errors with out-of-tree builds, e.g. > >>> > >>> ../drivers/gpu/drm/radeon/r420.c:38:10: fatal error: r420_reg_safe.h: No > >>> such file or directory > >>> 38 | #include "r420_reg_safe.h" > >>> | ^ > >> > >> Well stuff like that usually points to a broken build environment. > > Just a separate build directory. Specifically, I'm hitting the errors with > > > > make -C build-amd64 M=drivers/gpu/drm > > > > Generated headers such as r420_reg_safe.h reside in the build directory, so > > source files in the source directory can't find them without an explicit > > search path. > > I'm trying to swap back into my brain how all of this used to work, but > that's a really long time ago that I tried this as well. > > > Are you saying that should get added automagically somehow? > > Yes, exactly that. I'm like 95% sure that used to work, but I don't know > why exactly either. > > > FWIW, this is pretty common in the kernel according to git grep. > > Maybe Alex or somebody else with some more background in the kernel > Makefiles could jump in and help here. I don't remember either. I vaguely recall the build support for the mkregtable stuff being reworked a while ago. A quick zip through the git logs shows a series from Masahiro Yamada from 2020. Alex
Re: refactor the i915 GVT support and move to the modern mdev API v3
On Thu, Apr 14, 2022 at 12:20:42PM +, Wang, Zhi A wrote: > On 4/13/22 11:20 PM, Jason Gunthorpe wrote: > > On Wed, Apr 13, 2022 at 11:13:06PM +, Wang, Zhi A wrote: > >> Hi folks: > >> > >> Thanks so much for the efforts. I prepared a branch which contains all our > >> patches.The aim of the branch is for the VFIO maintainers to pull the > >> whole bunch easily after the drm-intel-next got merged through drm (as one > >> of the MMIO patches depends on a patch in drm-intel-next). > >> > >> I dropped patch 4 and patch 5 as they have been covered by Jani's patches. > >> Some conflicts was solved. > >> QA is going to test it today. > >> > >> You can find it here: > >> > >> git clone https://github.com/intel/gvt-linux -b for-christoph > > > > There are alot of extra commits on there - is it possible to base this > > straight on rc1 not on some kind of existing DRM tree? > > > > Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly > > from frontbuffer flush as a base? > > > > Jason > > > > Hi Jason: > > I updated the branch. You can check if those are what you are expecting. :) This is better, except for the first commit: [DON'T PULL] drm/i915/dmc: split out dmc registers to a separate file THIS PATCH WILL GO THROUGH DRM-INTEL-NEXT TO UPSTREAM Clean up the massive i915_reg.h a bit with this isolated set of registers. v2: Remove stale comment (Lucas) Clean the commit message and send that as a proper PR to drm-intel-next, then everything else is OK. Jason
Re: refactor the i915 GVT support and move to the modern mdev API v3
On 4/14/22 1:34 PM, Jason Gunthorpe wrote: > On Thu, Apr 14, 2022 at 12:20:42PM +, Wang, Zhi A wrote: >> On 4/13/22 11:20 PM, Jason Gunthorpe wrote: >>> On Wed, Apr 13, 2022 at 11:13:06PM +, Wang, Zhi A wrote: Hi folks: Thanks so much for the efforts. I prepared a branch which contains all our patches.The aim of the branch is for the VFIO maintainers to pull the whole bunch easily after the drm-intel-next got merged through drm (as one of the MMIO patches depends on a patch in drm-intel-next). I dropped patch 4 and patch 5 as they have been covered by Jani's patches. Some conflicts was solved. QA is going to test it today. You can find it here: git clone https://github.com/intel/gvt-linux -b for-christoph >>> >>> There are alot of extra commits on there - is it possible to base this >>> straight on rc1 not on some kind of existing DRM tree? >>> >>> Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly >>> from frontbuffer flush as a base? >>> >>> Jason >>> >> >> Hi Jason: >> This one belongs to i915, which has already been queued in drm-intel-next, but not yet reached to the top. When it is landed in -rc, I will rebase this branch on it, then we can drop this patch in this branch. >> I updated the branch. You can check if those are what you are expecting. :) > > This is better, except for the first commit: > > [DON'T PULL] drm/i915/dmc: split out dmc registers to a separate file > THIS PATCH WILL GO THROUGH DRM-INTEL-NEXT TO UPSTREAM > > Clean up the massive i915_reg.h a bit with this isolated set of > registers. > > v2: Remove stale comment (Lucas) > > Clean the commit message and send that as a proper PR to > drm-intel-next, then everything else is OK. > > Jason >
Re: refactor the i915 GVT support and move to the modern mdev API v3
On Thu, 14 Apr 2022, Jason Gunthorpe wrote: > On Thu, Apr 14, 2022 at 12:20:42PM +, Wang, Zhi A wrote: >> On 4/13/22 11:20 PM, Jason Gunthorpe wrote: >> > On Wed, Apr 13, 2022 at 11:13:06PM +, Wang, Zhi A wrote: >> >> Hi folks: >> >> >> >> Thanks so much for the efforts. I prepared a branch which contains all >> >> our patches.The aim of the branch is for the VFIO maintainers to pull the >> >> whole bunch easily after the drm-intel-next got merged through drm (as >> >> one of the MMIO patches depends on a patch in drm-intel-next). >> >> >> >> I dropped patch 4 and patch 5 as they have been covered by Jani's >> >> patches. Some conflicts was solved. >> >> QA is going to test it today. >> >> >> >> You can find it here: >> >> >> >> git clone https://github.com/intel/gvt-linux -b for-christoph >> > >> > There are alot of extra commits on there - is it possible to base this >> > straight on rc1 not on some kind of existing DRM tree? >> > >> > Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly >> > from frontbuffer flush as a base? >> > >> > Jason >> > >> >> Hi Jason: >> >> I updated the branch. You can check if those are what you are expecting. :) > > This is better, except for the first commit: > > [DON'T PULL] drm/i915/dmc: split out dmc registers to a separate file > THIS PATCH WILL GO THROUGH DRM-INTEL-NEXT TO UPSTREAM > > Clean up the massive i915_reg.h a bit with this isolated set of > registers. > > v2: Remove stale comment (Lucas) > > Clean the commit message and send that as a proper PR to > drm-intel-next, then everything else is OK. It's already in drm-intel-next, I guess the problem is basing the branch on something that doesn't have it. I'd probably just base everything cleanly on -rc1, and whoever does the merge between the two will need to account for the missing include in the result. It's just adding one line in the right place. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: refactor the i915 GVT support and move to the modern mdev API v3
On Thu, Apr 14, 2022 at 01:39:11PM +, Wang, Zhi A wrote: > On 4/14/22 1:34 PM, Jason Gunthorpe wrote: > > On Thu, Apr 14, 2022 at 12:20:42PM +, Wang, Zhi A wrote: > >> On 4/13/22 11:20 PM, Jason Gunthorpe wrote: > >>> On Wed, Apr 13, 2022 at 11:13:06PM +, Wang, Zhi A wrote: > Hi folks: > > Thanks so much for the efforts. I prepared a branch which contains all > our patches.The aim of the branch is for the VFIO maintainers to pull > the whole bunch easily after the drm-intel-next got merged through drm > (as one of the MMIO patches depends on a patch in drm-intel-next). > > I dropped patch 4 and patch 5 as they have been covered by Jani's > patches. Some conflicts was solved. > QA is going to test it today. > > You can find it here: > > git clone https://github.com/intel/gvt-linux -b for-christoph > >>> > >>> There are alot of extra commits on there - is it possible to base this > >>> straight on rc1 not on some kind of existing DRM tree? > >>> > >>> Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly > >>> from frontbuffer flush as a base? > >>> > >>> Jason > >>> > >> > >> Hi Jason: > >> > This one belongs to i915, which has already been queued in drm-intel-next, but > not yet reached to the top. When it is landed in -rc, I will rebase this > branch > on it, then we can drop this patch in this branch. A commit called 'split out dmc registers' with no Fixes: will be sent to a rc? Jason
Re: refactor the i915 GVT support and move to the modern mdev API v3
On Thu, Apr 14, 2022 at 04:40:11PM +0300, Jani Nikula wrote: > >> >> git clone https://github.com/intel/gvt-linux -b for-christoph > >> > > >> > There are alot of extra commits on there - is it possible to base this > >> > straight on rc1 not on some kind of existing DRM tree? > >> > > >> > Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly > >> > from frontbuffer flush as a base? > >> > > >> > Jason > >> > > >> > >> Hi Jason: > >> > >> I updated the branch. You can check if those are what you are expecting. :) > > > > This is better, except for the first commit: > > > > [DON'T PULL] drm/i915/dmc: split out dmc registers to a separate file > > THIS PATCH WILL GO THROUGH DRM-INTEL-NEXT TO UPSTREAM > > > > Clean up the massive i915_reg.h a bit with this isolated set of > > registers. > > > > v2: Remove stale comment (Lucas) > > > > Clean the commit message and send that as a proper PR to > > drm-intel-next, then everything else is OK. > > It's already in drm-intel-next, I guess the problem is basing the branch > on something that doesn't have it. I'd probably just base everything > cleanly on -rc1, and whoever does the merge between the two will need to > account for the missing include in the result. It's just adding one line > in the right place. That makes sense to me, especially if you can do the merge fixup internally in DRM. So drop the '[DONT PULL]' commit and send a PR to the next DRM tree - when that is confirmed send the same PR to vfio, Thanks, Jason
[PATCH 5.4 291/475] video: fbdev: nvidiafb: Use strscpy() to prevent buffer overflow
From: Tim Gardner
[ Upstream commit 37a1a2e6eeeb101285cd34e12e48a881524701aa ]
Coverity complains of a possible buffer overflow. However,
given the 'static' scope of nvidia_setup_i2c_bus() it looks
like that can't happen after examiniing the call sites.
CID 19036 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW)
1. fixed_size_dest: You might overrun the 48-character fixed-size string
chan->adapter.name by copying name without checking the length.
2. parameter_as_source: Note: This defect has an elevated risk because the
source argument is a parameter of the current function.
89strcpy(chan->adapter.name, name);
Fix this warning by using strscpy() which will silence the warning and
prevent any future buffer overflows should the names used to identify the
channel become much longer.
Cc: Antonino Daplas
Cc: linux-fb...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Tim Gardner
Signed-off-by: Helge Deller
Signed-off-by: Sasha Levin
---
drivers/video/fbdev/nvidia/nv_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/nvidia/nv_i2c.c
b/drivers/video/fbdev/nvidia/nv_i2c.c
index d7994a173245..0b48965a6420 100644
--- a/drivers/video/fbdev/nvidia/nv_i2c.c
+++ b/drivers/video/fbdev/nvidia/nv_i2c.c
@@ -86,7 +86,7 @@ static int nvidia_setup_i2c_bus(struct nvidia_i2c_chan *chan,
const char *name,
{
int rc;
- strcpy(chan->adapter.name, name);
+ strscpy(chan->adapter.name, name, sizeof(chan->adapter.name));
chan->adapter.owner = THIS_MODULE;
chan->adapter.class = i2c_class;
chan->adapter.algo_data = &chan->algo;
--
2.34.1
Re: refactor the i915 GVT support and move to the modern mdev API v3
On Thu, 14 Apr 2022, Jason Gunthorpe wrote: > On Thu, Apr 14, 2022 at 01:39:11PM +, Wang, Zhi A wrote: >> This one belongs to i915, which has already been queued in drm-intel-next, >> but >> not yet reached to the top. When it is landed in -rc, I will rebase this >> branch >> on it, then we can drop this patch in this branch. > > A commit called 'split out dmc registers' with no Fixes: will be sent > to a rc? Won't. It's in drm-intel-next (and drm-next), headed to v5.19. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 0/1] add support for enum module parameters
On Thu, Apr 14, 2022 at 03:30:32PM +0300, Jani Nikula wrote: > Hey, I've sent this before, ages ago, but haven't really followed > through with it. I still think it would be useful for many scenarios > where a plain number is a clumsy interface for a module param. > > Thoughts? We should not be adding new module parameters anyway (they operate on code, not data/devices), so what would this be used for? thanks, greg k-h
Re: [PATCH v2 2/5] drm/i915: sanitize mem_flags for stolen buffers
On Tue, 2022-04-12 at 15:18 +, Robert Beckett wrote: > stolen regions are not page backed or considered iomem. thus... A little more elaborate commit message please. > > Signed-off-by: Robert Beckett Reviewed-by: Thomas Hellström > --- > drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c > b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c > index 358f8a1a30ce..9fe8132de3b2 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c > @@ -122,8 +122,9 @@ void i915_ttm_adjust_gem_after_move(struct > drm_i915_gem_object *obj) > > obj->mem_flags &= ~(I915_BO_FLAG_STRUCT_PAGE | > I915_BO_FLAG_IOMEM); > > - obj->mem_flags |= i915_ttm_cpu_maps_iomem(bo->resource) ? > I915_BO_FLAG_IOMEM : > - I915_BO_FLAG_STRUCT_PAGE; > + if (obj->mm.region->id != INTEL_REGION_STOLEN_SMEM) > + obj->mem_flags |= i915_ttm_cpu_maps_iomem(bo- > >resource) ? I915_BO_FLAG_IOMEM : > + I915_BO_FLAG_STRUCT_PAGE; > > cache_level = i915_ttm_cache_level(to_i915(bo->base.dev), bo- > >resource, > bo->ttm);
Re: [PATCH v2 3/5] drm/i915: ttm move/clear logic fix
On Tue, 2022-04-12 at 15:18 +, Robert Beckett wrote:
> ttm managed buffers start off with system resource definitions and
> ttm_tt
> tracking structures allocated (though unpopulated).
> currently this prevents clearing of buffers on first move to desired
> placements.
>
> The desired behaviour is to clear user allocated buffers and any
> kernel
> buffers that specifically requests it only.
> Make the logic match the desired behaviour.
>
> Signed-off-by: Robert Beckett
Reviewed-by: Thomas Hellström
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 22
> +++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> index 9fe8132de3b2..9cf85f91edb5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> @@ -3,6 +3,7 @@
> * Copyright © 2021 Intel Corporation
> */
>
> +#include "drm/ttm/ttm_tt.h"
> #include
>
> #include "i915_deps.h"
> @@ -470,6 +471,25 @@ __i915_ttm_move(struct ttm_buffer_object *bo,
> return fence;
> }
>
> +static bool
> +allow_clear(struct drm_i915_gem_object *obj, struct ttm_tt *ttm,
> struct ttm_resource *dst_mem)
> +{
> + /* never clear stolen */
> + if (dst_mem->mem_type == I915_PL_STOLEN)
> + return false;
> + /*
> + * we want to clear user buffers and any kernel buffers
> + * that specifically request clearing.
> + */
> + if (obj->flags & I915_BO_ALLOC_USER)
> + return true;
> +
> + if (ttm && ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC)
> + return true;
> +
> + return false;
> +}
> +
> /**
> * i915_ttm_move - The TTM move callback used by i915.
> * @bo: The buffer object.
> @@ -520,7 +540,7 @@ int i915_ttm_move(struct ttm_buffer_object *bo,
> bool evict,
> return PTR_ERR(dst_rsgt);
>
> clear = !i915_ttm_cpu_maps_iomem(bo->resource) && (!ttm ||
> !ttm_tt_is_populated(ttm));
> - if (!(clear && ttm && !(ttm->page_flags &
> TTM_TT_FLAG_ZERO_ALLOC))) {
> + if (!clear || allow_clear(obj, ttm, dst_mem)) {
> struct i915_deps deps;
>
> i915_deps_init(&deps, GFP_KERNEL | __GFP_NORETRY |
> __GFP_NOWARN);
Re: [PATCH v2 4/5] drm/i915: ttm backend dont provide mmap_offset for kernel buffers
On Tue, 2022-04-12 at 15:18 +, Robert Beckett wrote:
> stolen/kernel buffers should not be mmapable by userland.
> do not provide callbacks to facilitate this for these buffers.
>
> Signed-off-by: Robert Beckett
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 32 +--
> --
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index a878910a563c..b20f81836c54 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -1092,8 +1092,8 @@ static void i915_ttm_unmap_virtual(struct
> drm_i915_gem_object *obj)
> ttm_bo_unmap_virtual(i915_gem_to_ttm(obj));
> }
>
> -static const struct drm_i915_gem_object_ops i915_gem_ttm_obj_ops = {
> - .name = "i915_gem_object_ttm",
> +static const struct drm_i915_gem_object_ops
> i915_gem_ttm_user_obj_ops = {
> + .name = "i915_gem_object_ttm_user",
> .flags = I915_GEM_OBJECT_IS_SHRINKABLE |
> I915_GEM_OBJECT_SELF_MANAGED_SHRINK_LIST,
>
> @@ -,6 +,21 @@ static const struct drm_i915_gem_object_ops
> i915_gem_ttm_obj_ops = {
> .mmap_ops = &vm_ops_ttm,
> };
>
> +static const struct drm_i915_gem_object_ops
> i915_gem_ttm_kern_obj_ops = {
> + .name = "i915_gem_object_ttm_kern",
> + .flags = I915_GEM_OBJECT_IS_SHRINKABLE |
> + I915_GEM_OBJECT_SELF_MANAGED_SHRINK_LIST,
> +
> + .get_pages = i915_ttm_get_pages,
> + .put_pages = i915_ttm_put_pages,
> + .truncate = i915_ttm_truncate,
> + .shrink = i915_ttm_shrink,
> +
> + .adjust_lru = i915_ttm_adjust_lru,
> + .delayed_free = i915_ttm_delayed_free,
> + .migrate = i915_ttm_migrate,
> +};
Do we really need two different ops here?
Since if we don't have mmap ops, basically that tells GEM it should do
the mmapping rather than TTM.
That might of course come in handy for the shmem backend, but I don't
fully follow why we need this for stolen.
Also for the framebuffer handed over from BIOS to fbdev, Does that need
mmapping and if so, how do we handle that?
/Thomas
> +
> void i915_ttm_bo_destroy(struct ttm_buffer_object *bo)
> {
> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> @@ -1165,10 +1180,19 @@ int __i915_gem_ttm_object_init(struct
> intel_memory_region *mem,
> .no_wait_gpu = false,
> };
> enum ttm_bo_type bo_type;
> + const struct drm_i915_gem_object_ops *ops;
> int ret;
>
> drm_gem_private_object_init(&i915->drm, &obj->base, size);
> - i915_gem_object_init(obj, &i915_gem_ttm_obj_ops, &lock_class,
> flags);
> +
> + if (flags & I915_BO_ALLOC_USER &&
> intel_region_to_ttm_type(mem) != I915_PL_STOLEN) {
> + bo_type = ttm_bo_type_device;
> + ops = &i915_gem_ttm_user_obj_ops;
> + } else {
> + bo_type = ttm_bo_type_kernel;
> + ops = &i915_gem_ttm_kern_obj_ops;
> + }
> + i915_gem_object_init(obj, ops, &lock_class, flags);
>
> obj->bo_offset = offset;
>
> @@ -1178,8 +1202,6 @@ int __i915_gem_ttm_object_init(struct
> intel_memory_region *mem,
>
> INIT_RADIX_TREE(&obj->ttm.get_io_page.radix, GFP_KERNEL |
> __GFP_NOWARN);
> mutex_init(&obj->ttm.get_io_page.lock);
> - bo_type = (obj->flags & I915_BO_ALLOC_USER) ?
> ttm_bo_type_device :
> - ttm_bo_type_kernel;
>
> obj->base.vma_node.driver_private = i915_gem_to_ttm(obj);
>
Re: [CI v3 00/12] drm/edid: low level EDID block read refactoring etc.
On Mon, 11 Apr 2022, Jani Nikula wrote: > v3 of [1] fixing the (!x == 0) mistake. Resending the entire thing with > hopes the test robot picks up the baseline better; I haven't had much > luck with --in-reply-to for that. There were some issues with our CI, but I'm pretty sure they were not caused by the series. Fingers crossed. Thanks for the reviews, pushed. BR, Jani. > > BR, > Jani. > > [1] https://patchwork.freedesktop.org/series/102329/ > > Jani Nikula (12): > drm/edid: convert edid_is_zero() to edid_block_is_zero() for blocks > drm/edid: have edid_block_check() detect blocks that are all zero > drm/edid: refactor EDID block status printing > drm/edid: add a helper to log dump an EDID block > drm/edid: pass struct edid to connector_bad_edid() > drm/edid: add typedef for block read function > drm/edid: abstract an EDID block read helper > drm/edid: use EDID block read helper in drm_do_get_edid() > drm/edid: convert extension block read to EDID block read helper > drm/edid: drop extra local var > drm/edid: add single point of return to drm_do_get_edid() > drm/edid: add EDID block count and size helpers > > drivers/gpu/drm/drm_edid.c | 350 - > 1 file changed, 225 insertions(+), 125 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH 9/9] vfio: Remove calls to vfio_group_add_container_user()
On Thu, Apr 14, 2022 at 09:51:49AM -0400, Matthew Rosato wrote: > On 4/12/22 11:53 AM, Jason Gunthorpe wrote: > > When the open_device() op is called the container_users is incremented and > > held incremented until close_device(). Thus, so long as drivers call > > functions within their open_device()/close_device() region they do not > > need to worry about the container_users. > > > > These functions can all only be called between > > open_device()/close_device(): > > > >vfio_pin_pages() > >vfio_unpin_pages() > >vfio_dma_rw() > >vfio_register_notifier() > >vfio_unregister_notifier() > > > > So eliminate the calls to vfio_group_add_container_user() and add a simple > > WARN_ON to detect mis-use by drivers. > > > > vfio_device_fops_release decrements dev->open_count immediately before > calling dev->ops->close_device, which means we could enter close_device with > a dev_count of 0. > > Maybe vfio_device_fops_release should handle the same way as > vfio_group_get_device_fd? > > if (device->open_count == 1 && device->ops->close_device) > device->ops->close_device(device); > device->open_count--; Yes, thanks alot! I have nothing to test these flows on... It matches the ordering in the only other place to call close_device. I folded this into the patch: diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 0f735f9f206002..29761f0cf0a227 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -1551,8 +1551,9 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) mutex_lock(&device->dev_set->lock); vfio_assert_device_open(device); - if (!--device->open_count && device->ops->close_device) + if (device->open_count == 1 && device->ops->close_device) device->ops->close_device(device); + device->open_count--; mutex_unlock(&device->dev_set->lock); module_put(device->dev->driver->owner); Jason
Re: [PATCH 0/1] add support for enum module parameters
On Thu, 14 Apr 2022, Greg Kroah-Hartman wrote: > On Thu, Apr 14, 2022 at 03:30:32PM +0300, Jani Nikula wrote: >> Hey, I've sent this before, ages ago, but haven't really followed >> through with it. I still think it would be useful for many scenarios >> where a plain number is a clumsy interface for a module param. >> >> Thoughts? > > We should not be adding new module parameters anyway (they operate on > code, not data/devices), so what would this be used for? I think it's just easier to use names than random values, and this also gives you range check on the input. I also keep telling people not to add new module parameters, but it's not like they're going away anytime soon. If there's a solution to being able to pass device specific debug parameters at probe time, I'm all ears. At least i915 has a bunch of things which can't really be changed after probe, when debugfs for the device is around. Module parameters aren't ideal, but debugfs doesn't work for this. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: refactor the i915 GVT support and move to the modern mdev API v3
On 4/14/22 1:43 PM, Jason Gunthorpe wrote: > On Thu, Apr 14, 2022 at 04:40:11PM +0300, Jani Nikula wrote: > >> git clone https://github.com/intel/gvt-linux -b for-christoph > > There are alot of extra commits on there - is it possible to base this > straight on rc1 not on some kind of existing DRM tree? > > Why did you choose drm/i915/fbc: Call intel_fbc_activate() directly > from frontbuffer flush as a base? > > Jason > Hi Jason: I updated the branch. You can check if those are what you are expecting. :) >>> >>> This is better, except for the first commit: >>> >>> [DON'T PULL] drm/i915/dmc: split out dmc registers to a separate file >>> THIS PATCH WILL GO THROUGH DRM-INTEL-NEXT TO UPSTREAM >>> >>> Clean up the massive i915_reg.h a bit with this isolated set of >>> registers. >>> >>> v2: Remove stale comment (Lucas) >>> >>> Clean the commit message and send that as a proper PR to >>> drm-intel-next, then everything else is OK. >> >> It's already in drm-intel-next, I guess the problem is basing the branch >> on something that doesn't have it. I'd probably just base everything >> cleanly on -rc1, and whoever does the merge between the two will need to >> account for the missing include in the result. It's just adding one line >> in the right place. > > That makes sense to me, especially if you can do the merge fixup > internally in DRM. > > So drop the '[DONT PULL]' commit and send a PR to the next DRM tree - > when that is confirmed send the same PR to vfio, I updated the branch again, but I am confused. What is the purpose of sending the PR to next DRM tree? I suppose all the patches will go through VFIO? If I understand correctly? > > Thanks, > Jason >
Re: [PATCH 0/1] add support for enum module parameters
On Thu, Apr 14, 2022 at 05:22:47PM +0300, Jani Nikula wrote: > On Thu, 14 Apr 2022, Greg Kroah-Hartman wrote: > > On Thu, Apr 14, 2022 at 03:30:32PM +0300, Jani Nikula wrote: > >> Hey, I've sent this before, ages ago, but haven't really followed > >> through with it. I still think it would be useful for many scenarios > >> where a plain number is a clumsy interface for a module param. > >> > >> Thoughts? > > > > We should not be adding new module parameters anyway (they operate on > > code, not data/devices), so what would this be used for? > > I think it's just easier to use names than random values, and this also > gives you range check on the input. > > I also keep telling people not to add new module parameters, but it's > not like they're going away anytime soon. Existing ones can not go away (or change), but we do not have to add new ones. > If there's a solution to being able to pass device specific debug > parameters at probe time, I'm all ears. At least i915 has a bunch of > things which can't really be changed after probe, when debugfs for the > device is around. Module parameters aren't ideal, but debugfs doesn't > work for this. configfs?
Re: refactor the i915 GVT support and move to the modern mdev API v3
On Thu, Apr 14, 2022 at 02:25:36PM +, Wang, Zhi A wrote: > > So drop the '[DONT PULL]' commit and send a PR to the next DRM tree - > > when that is confirmed send the same PR to vfio, > > I updated the branch again, but I am confused. What is the purpose of sending > the PR to next DRM tree? I suppose all the patches will go through VFIO? If > I understand correctly? pull requests can flow through more than one tree concurrently. The purpose of the topic branch is to allow all the commits to be in all the trees they need to be in at once. So you should send this branch as a PR to the next logical upstream tree gvt patches normally go through, in the usual way that you send PRs. Especially in this case where there is a small merge conflict internal to DRM to resolve. I'm assuming this is the drm-intel-next tree? Once DRM is internally happy then VFIO can merge it as well. You can view VFIO as the secondary path to Linus, DRM as primary. Alex will mention in his pull request that VFIO has a 'shared branch with DRM for gvt'. Jason
Re: [PATCH v3 5/5] drm/i915: stolen memory use ttm backend
Hi,
On 4/12/22 21:38, Robert Beckett wrote:
+struct ttm_resource *
+i915_gem_stolen_reserve_range(struct drm_i915_private *i915,
+ resource_size_t size,
+ u64 start, u64 end)
{
- int ret;
+ struct intel_memory_region *mem = i915->mm.stolen_region;
- if (!drm_mm_initialized(&i915->mm.stolen))
- return -ENODEV;
-
- /* WaSkipStolenMemoryFirstPage:bdw+ */
- if (GRAPHICS_VER(i915) >= 8 && start < 4096)
- start = 4096;
Did we lose this Workaround?
Need to continue reviewing this patchnext week.
Thx,
Thomas
-
- mutex_lock(&i915->mm.stolen_lock);
- ret = drm_mm_insert_node_in_range(&i915->mm.stolen, node,
- size, alignment, 0,
- start, end, DRM_MM_INSERT_BEST);
- mutex_unlock(&i915->mm.stolen_lock);
+ if (!mem)
+ return ERR_PTR(-ENODEV);
+ return intel_region_ttm_resource_alloc(mem, size, start, end,
I915_BO_ALLOC_CONTIGUOUS);
+}
-
[PATCH 01/19] drm/edid: reset display info in drm_add_edid_modes() for NULL edid
If a NULL edid gets passed to drm_add_edid_modes(), we should probably
also reset the display info.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 324ce8467915..4758e78fad82 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5721,6 +5721,7 @@ static int drm_edid_connector_update(struct drm_connector
*connector,
u32 quirks;
if (edid == NULL) {
+ drm_reset_display_info(connector);
clear_eld(connector);
return 0;
}
--
2.30.2
[PATCH 00/19] drm/edid: CEA data block iterators, and more
Get back to the CEA data block iterator stuff now that a bunch of prep work has landed. My git repo at [1], branch edid-hfeeodb-2022-04-14, contains where this is all headed after this. [1] https://cgit.freedesktop.org/~jani/drm/log/?h=edid-hfeeodb-2022-04-14 Jani Nikula (18): drm/edid: reset display info in drm_add_edid_modes() for NULL edid drm/edid: clean up CTA data block tag definitions drm/edid: add iterator for EDID base and extension blocks drm/edid: add iterator for CTA data blocks drm/edid: clean up cea_db_is_*() functions drm/edid: convert add_cea_modes() to use cea db iter drm/edid: convert drm_edid_to_speaker_allocation() to use cea db iter drm/edid: convert drm_edid_to_sad() to use cea db iter drm/edid: convert drm_detect_hdmi_monitor() to use cea db iter drm/edid: convert drm_detect_monitor_audio() to use cea db iter drm/edid: convert drm_parse_cea_ext() to use cea db iter drm/edid: convert drm_edid_to_eld() to use cea db iter drm/edid: sunset the old unused cea data block iterators drm/edid: restore some type safety to cea_db_*() functions drm/edid: detect basic audio in all CEA extensions drm/edid: detect color formats and CTA revision in all CTA extensions drm/edid: skip CTA extension scan in drm_edid_to_eld() just for CTA rev drm/edid: sunset drm_find_cea_extension() Lee Shawn C (1): drm/edid: check for HF-SCDB block drivers/gpu/drm/drm_edid.c | 760 + 1 file changed, 438 insertions(+), 322 deletions(-) -- 2.30.2
[PATCH 02/19] drm/edid: check for HF-SCDB block
From: Lee Shawn C
Find HF-SCDB information in CEA extensions block. And retrieve
Max_TMDS_Character_Rate that support by sink device.
v2: HF-SCDB and HF-VSDBS carry the same SCDS data. Reuse
drm_parse_hdmi_forum_vsdb() to parse this packet.
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Ankit Nautiyal
Cc: intel-gfx
Signed-off-by: Lee Shawn C
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 4758e78fad82..32ece9607b94 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3495,6 +3495,7 @@ add_detailed_modes(struct drm_connector *connector, const
struct edid *edid,
#define EXT_VIDEO_CAPABILITY_BLOCK 0x00
#define EXT_VIDEO_DATA_BLOCK_420 0x0E
#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
+#define EXT_VIDEO_HF_SCDB_DATA_BLOCK 0x79
#define EDID_BASIC_AUDIO (1 << 6)
#define EDID_CEA_YCRCB444 (1 << 5)
#define EDID_CEA_YCRCB422 (1 << 4)
@@ -4426,6 +4427,20 @@ static bool cea_db_is_vcdb(const u8 *db)
return true;
}
+static bool cea_db_is_hdmi_forum_scdb(const u8 *db)
+{
+ if (cea_db_tag(db) != USE_EXTENDED_TAG)
+ return false;
+
+ if (cea_db_payload_len(db) < 7)
+ return false;
+
+ if (cea_db_extended_tag(db) != EXT_VIDEO_HF_SCDB_DATA_BLOCK)
+ return false;
+
+ return true;
+}
+
static bool cea_db_is_y420cmdb(const u8 *db)
{
if (cea_db_tag(db) != USE_EXTENDED_TAG)
@@ -5387,7 +5402,8 @@ static void drm_parse_cea_ext(struct drm_connector
*connector,
if (cea_db_is_hdmi_vsdb(db))
drm_parse_hdmi_vsdb_video(connector, db);
- if (cea_db_is_hdmi_forum_vsdb(db))
+ if (cea_db_is_hdmi_forum_vsdb(db) ||
+ cea_db_is_hdmi_forum_scdb(db))
drm_parse_hdmi_forum_vsdb(connector, db);
if (cea_db_is_microsoft_vsdb(db))
drm_parse_microsoft_vsdb(connector, db);
--
2.30.2
[PATCH 03/19] drm/edid: clean up CTA data block tag definitions
Add prefixed names, group, sort, add references.
v2:
- Updated references to CTA-861-H
- s/CEA/CTA/ in data block macros
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/drm_edid.c | 65 --
1 file changed, 35 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 32ece9607b94..18b0ff223885 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3486,16 +3486,21 @@ add_detailed_modes(struct drm_connector *connector,
const struct edid *edid,
return closure.modes;
}
-#define AUDIO_BLOCK0x01
-#define VIDEO_BLOCK 0x02
-#define VENDOR_BLOCK0x03
-#define SPEAKER_BLOCK 0x04
-#define HDR_STATIC_METADATA_BLOCK 0x6
-#define USE_EXTENDED_TAG 0x07
-#define EXT_VIDEO_CAPABILITY_BLOCK 0x00
-#define EXT_VIDEO_DATA_BLOCK_420 0x0E
-#define EXT_VIDEO_CAP_BLOCK_Y420CMDB 0x0F
-#define EXT_VIDEO_HF_SCDB_DATA_BLOCK 0x79
+/* CTA-861-H Table 60 - CTA Tag Codes */
+#define CTA_DB_AUDIO 1
+#define CTA_DB_VIDEO 2
+#define CTA_DB_VENDOR 3
+#define CTA_DB_SPEAKER 4
+#define CTA_DB_EXTENDED_TAG7
+
+/* CTA-861-H Table 62 - CTA Extended Tag Codes */
+#define CTA_EXT_DB_VIDEO_CAP 0
+#define CTA_EXT_DB_VENDOR 1
+#define CTA_EXT_DB_HDR_STATIC_METADATA 6
+#define CTA_EXT_DB_420_VIDEO_DATA 14
+#define CTA_EXT_DB_420_VIDEO_CAP_MAP 15
+#define CTA_EXT_DB_HF_SCDB 0x79
+
#define EDID_BASIC_AUDIO (1 << 6)
#define EDID_CEA_YCRCB444 (1 << 5)
#define EDID_CEA_YCRCB422 (1 << 4)
@@ -4382,7 +4387,7 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
static bool cea_db_is_hdmi_vsdb(const u8 *db)
{
- if (cea_db_tag(db) != VENDOR_BLOCK)
+ if (cea_db_tag(db) != CTA_DB_VENDOR)
return false;
if (cea_db_payload_len(db) < 5)
@@ -4393,7 +4398,7 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
{
- if (cea_db_tag(db) != VENDOR_BLOCK)
+ if (cea_db_tag(db) != CTA_DB_VENDOR)
return false;
if (cea_db_payload_len(db) < 7)
@@ -4404,7 +4409,7 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
static bool cea_db_is_microsoft_vsdb(const u8 *db)
{
- if (cea_db_tag(db) != VENDOR_BLOCK)
+ if (cea_db_tag(db) != CTA_DB_VENDOR)
return false;
if (cea_db_payload_len(db) != 21)
@@ -4415,13 +4420,13 @@ static bool cea_db_is_microsoft_vsdb(const u8 *db)
static bool cea_db_is_vcdb(const u8 *db)
{
- if (cea_db_tag(db) != USE_EXTENDED_TAG)
+ if (cea_db_tag(db) != CTA_DB_EXTENDED_TAG)
return false;
if (cea_db_payload_len(db) != 2)
return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_CAPABILITY_BLOCK)
+ if (cea_db_extended_tag(db) != CTA_EXT_DB_VIDEO_CAP)
return false;
return true;
@@ -4429,13 +4434,13 @@ static bool cea_db_is_vcdb(const u8 *db)
static bool cea_db_is_hdmi_forum_scdb(const u8 *db)
{
- if (cea_db_tag(db) != USE_EXTENDED_TAG)
+ if (cea_db_tag(db) != CTA_DB_EXTENDED_TAG)
return false;
if (cea_db_payload_len(db) < 7)
return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_HF_SCDB_DATA_BLOCK)
+ if (cea_db_extended_tag(db) != CTA_EXT_DB_HF_SCDB)
return false;
return true;
@@ -4443,13 +4448,13 @@ static bool cea_db_is_hdmi_forum_scdb(const u8 *db)
static bool cea_db_is_y420cmdb(const u8 *db)
{
- if (cea_db_tag(db) != USE_EXTENDED_TAG)
+ if (cea_db_tag(db) != CTA_DB_EXTENDED_TAG)
return false;
if (!cea_db_payload_len(db))
return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_CAP_BLOCK_Y420CMDB)
+ if (cea_db_extended_tag(db) != CTA_EXT_DB_420_VIDEO_CAP_MAP)
return false;
return true;
@@ -4457,13 +4462,13 @@ static bool cea_db_is_y420cmdb(const u8 *db)
static bool cea_db_is_y420vdb(const u8 *db)
{
- if (cea_db_tag(db) != USE_EXTENDED_TAG)
+ if (cea_db_tag(db) != CTA_DB_EXTENDED_TAG)
return false;
if (!cea_db_payload_len(db))
return false;
- if (cea_db_extended_tag(db) != EXT_VIDEO_DATA_BLOCK_420)
+ if (cea_db_extended_tag(db) != CTA_EXT_DB_420_VIDEO_DATA)
return false;
return true;
@@ -4530,7 +4535,7 @@ add_cea_modes(struct drm_connector *connector, const
struct edid *edid)
db = &cea[i];
dbl = cea_db_payload_len(db);
- if (cea_db_tag(db) == VIDEO_BLOCK) {
+ if (cea_db_tag(db) == CTA_DB_VIDEO) {
video = db + 1;
video_len =
[PATCH 05/19] drm/edid: add iterator for CTA data blocks
Add an iterator for CTA Data Blocks across EDID CTA Extensions and
DisplayID CTA Data Blocks.
v2: Update references, note why we can trust displayid ranges (Ville)
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 202 ++---
1 file changed, 190 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 40ffd7aba157..43d9e04f8fb9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4360,24 +4360,12 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len,
return modes;
}
-static int
-cea_db_payload_len(const u8 *db)
-{
- return db[0] & 0x1f;
-}
-
static int
cea_db_extended_tag(const u8 *db)
{
return db[1];
}
-static int
-cea_db_tag(const u8 *db)
-{
- return db[0] >> 5;
-}
-
static int
cea_revision(const u8 *cea)
{
@@ -4433,6 +4421,196 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
return 0;
}
+/*
+ * CTA Data Block iterator.
+ *
+ * Iterate through all CTA Data Blocks in both EDID CTA Extensions and
DisplayID
+ * CTA Data Blocks.
+ *
+ * struct cea_db *db:
+ * struct cea_db_iter iter;
+ *
+ * cea_db_iter_edid_begin(edid, &iter);
+ * cea_db_iter_for_each(db, &iter) {
+ * // do stuff with db
+ * }
+ * cea_db_iter_end(&iter);
+ */
+struct cea_db_iter {
+ struct drm_edid_iter edid_iter;
+ struct displayid_iter displayid_iter;
+
+ /* Current Data Block Collection. */
+ const u8 *collection;
+
+ /* Current Data Block index in current collection. */
+ int index;
+
+ /* End index in current collection. */
+ int end;
+};
+
+/* CTA-861-H section 7.4 CTA Data BLock Collection */
+struct cea_db {
+ u8 tag_length;
+ u8 data[];
+} __packed;
+
+static int cea_db_tag(const void *_db)
+{
+ /* FIXME: Transition to passing struct cea_db * everywhere. */
+ const struct cea_db *db = _db;
+
+ return db->tag_length >> 5;
+}
+
+static int cea_db_payload_len(const void *_db)
+{
+ /* FIXME: Transition to passing struct cea_db * everywhere. */
+ const struct cea_db *db = _db;
+
+ return db->tag_length & 0x1f;
+}
+
+static const void *cea_db_data(const struct cea_db *db)
+{
+ return db->data;
+}
+
+static void cea_db_iter_edid_begin(const struct edid *edid, struct cea_db_iter
*iter)
+{
+ memset(iter, 0, sizeof(*iter));
+
+ drm_edid_iter_begin(edid, &iter->edid_iter);
+ displayid_iter_edid_begin(edid, &iter->displayid_iter);
+}
+
+static const struct cea_db *
+__cea_db_iter_current_block(const struct cea_db_iter *iter)
+{
+ const struct cea_db *db;
+
+ if (!iter->collection)
+ return NULL;
+
+ db = (const struct cea_db *)&iter->collection[iter->index];
+
+ if (iter->index + sizeof(*db) <= iter->end &&
+ iter->index + sizeof(*db) + cea_db_payload_len(db) <= iter->end)
+ return db;
+
+ return NULL;
+}
+
+/*
+ * References:
+ * - VESA E-EDID v1.4
+ * - CTA-861-H section 7.3.3 CTA Extension Version 3
+ */
+static const void *__cea_db_iter_edid_next(struct cea_db_iter *iter)
+{
+ const u8 *ext;
+
+ drm_edid_iter_for_each(ext, &iter->edid_iter) {
+ /* Only support CTA Extension revision 3+ */
+ if (ext[0] != CEA_EXT || cea_revision(ext) < 3)
+ continue;
+
+ iter->index = 4;
+ iter->end = ext[2];
+ if (iter->end == 0)
+ iter->end = 127;
+ if (iter->end < 4 || iter->end > 127)
+ continue;
+
+ return ext;
+ }
+
+ return NULL;
+}
+
+/*
+ * References:
+ * - DisplayID v1.3 Appendix C: CEA Data Block within a DisplayID Data Block
+ * - DisplayID v2.0 section 4.10 CTA DisplayID Data Block
+ *
+ * Note that the above do not specify any connection between DisplayID Data
+ * Block revision and CTA Extension versions.
+ */
+static const void *__cea_db_iter_displayid_next(struct cea_db_iter *iter)
+{
+ const struct displayid_block *block;
+
+ displayid_iter_for_each(block, &iter->displayid_iter) {
+ if (block->tag != DATA_BLOCK_CTA)
+ continue;
+
+ /*
+* The displayid iterator has already verified the block bounds
+* in displayid_iter_block().
+*/
+ iter->index = sizeof(*block);
+ iter->end = iter->index + block->num_bytes;
+
+ return block;
+ }
+
+ return NULL;
+}
+
+static const struct cea_db *__cea_db_iter_next(struct cea_db_iter *iter)
+{
+ const struct cea_db *db;
+
+ if (iter->collection) {
+ /* Current collection should always be valid. */
+ db = __cea_db_iter_current_block(iter);
+ if (WARN_ON(!db)) {
+ iter->collection
[PATCH 06/19] drm/edid: clean up cea_db_is_*() functions
Abstract helpers for matching vendor data blocks and extended tags, and
use them to simplify all the cea_db_is_*() functions.
Take void pointer as parameter to allow transitional use for both u8 *
and struct cea_db *.
v2: Remove superfluous parens (Ville)
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 127 -
1 file changed, 40 insertions(+), 87 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 43d9e04f8fb9..34897b1417a5 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4360,12 +4360,6 @@ do_hdmi_vsdb_modes(struct drm_connector *connector,
const u8 *db, u8 len,
return modes;
}
-static int
-cea_db_extended_tag(const u8 *db)
-{
- return db[1];
-}
-
static int
cea_revision(const u8 *cea)
{
@@ -4477,6 +4471,22 @@ static const void *cea_db_data(const struct cea_db *db)
return db->data;
}
+static bool cea_db_is_extended_tag(const struct cea_db *db, int tag)
+{
+ return cea_db_tag(db) == CTA_DB_EXTENDED_TAG &&
+ cea_db_payload_len(db) >= 1 &&
+ db->data[0] == tag;
+}
+
+static bool cea_db_is_vendor(const struct cea_db *db, int vendor_oui)
+{
+ const u8 *data = cea_db_data(db);
+
+ return cea_db_tag(db) == CTA_DB_VENDOR &&
+ cea_db_payload_len(db) >= 3 &&
+ oui(data[2], data[1], data[0]) == vendor_oui;
+}
+
static void cea_db_iter_edid_begin(const struct edid *edid, struct cea_db_iter
*iter)
{
memset(iter, 0, sizeof(*iter));
@@ -4611,93 +4621,50 @@ static void cea_db_iter_end(struct cea_db_iter *iter)
memset(iter, 0, sizeof(*iter));
}
-static bool cea_db_is_hdmi_vsdb(const u8 *db)
+static bool cea_db_is_hdmi_vsdb(const void *db)
{
- if (cea_db_tag(db) != CTA_DB_VENDOR)
- return false;
-
- if (cea_db_payload_len(db) < 5)
- return false;
-
- return oui(db[3], db[2], db[1]) == HDMI_IEEE_OUI;
+ return cea_db_is_vendor(db, HDMI_IEEE_OUI) &&
+ cea_db_payload_len(db) >= 5;
}
-static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
+static bool cea_db_is_hdmi_forum_vsdb(const void *db)
{
- if (cea_db_tag(db) != CTA_DB_VENDOR)
- return false;
-
- if (cea_db_payload_len(db) < 7)
- return false;
-
- return oui(db[3], db[2], db[1]) == HDMI_FORUM_IEEE_OUI;
+ return cea_db_is_vendor(db, HDMI_FORUM_IEEE_OUI) &&
+ cea_db_payload_len(db) >= 7;
}
-static bool cea_db_is_microsoft_vsdb(const u8 *db)
+static bool cea_db_is_microsoft_vsdb(const void *db)
{
- if (cea_db_tag(db) != CTA_DB_VENDOR)
- return false;
-
- if (cea_db_payload_len(db) != 21)
- return false;
-
- return oui(db[3], db[2], db[1]) == MICROSOFT_IEEE_OUI;
+ return cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) &&
+ cea_db_payload_len(db) == 21;
}
-static bool cea_db_is_vcdb(const u8 *db)
+static bool cea_db_is_vcdb(const void *db)
{
- if (cea_db_tag(db) != CTA_DB_EXTENDED_TAG)
- return false;
-
- if (cea_db_payload_len(db) != 2)
- return false;
-
- if (cea_db_extended_tag(db) != CTA_EXT_DB_VIDEO_CAP)
- return false;
-
- return true;
+ return cea_db_is_extended_tag(db, CTA_EXT_DB_VIDEO_CAP) &&
+ cea_db_payload_len(db) == 2;
}
-static bool cea_db_is_hdmi_forum_scdb(const u8 *db)
+static bool cea_db_is_hdmi_forum_scdb(const void *db)
{
- if (cea_db_tag(db) != CTA_DB_EXTENDED_TAG)
- return false;
-
- if (cea_db_payload_len(db) < 7)
- return false;
-
- if (cea_db_extended_tag(db) != CTA_EXT_DB_HF_SCDB)
- return false;
-
- return true;
+ return cea_db_is_extended_tag(db, CTA_EXT_DB_HF_SCDB) &&
+ cea_db_payload_len(db) >= 7;
}
-static bool cea_db_is_y420cmdb(const u8 *db)
+static bool cea_db_is_y420cmdb(const void *db)
{
- if (cea_db_tag(db) != CTA_DB_EXTENDED_TAG)
- return false;
-
- if (!cea_db_payload_len(db))
- return false;
-
- if (cea_db_extended_tag(db) != CTA_EXT_DB_420_VIDEO_CAP_MAP)
- return false;
-
- return true;
+ return cea_db_is_extended_tag(db, CTA_EXT_DB_420_VIDEO_CAP_MAP);
}
-static bool cea_db_is_y420vdb(const u8 *db)
+static bool cea_db_is_y420vdb(const void *db)
{
- if (cea_db_tag(db) != CTA_DB_EXTENDED_TAG)
- return false;
-
- if (!cea_db_payload_len(db))
- return false;
-
- if (cea_db_extended_tag(db) != CTA_EXT_DB_420_VIDEO_DATA)
- return false;
+ return cea_db_is_extended_tag(db, CTA_EXT_DB_420_VIDEO_DATA);
+}
- return true;
+static bool cea_db_is_hdmi_hdr_metadata_block(const void *db)
+{
+ return cea_db_is_extended_tag(db, CTA_EXT_DB_HDR_STATIC_METADATA) &&
+
[PATCH 04/19] drm/edid: add iterator for EDID base and extension blocks
Add an iterator abstraction for going through all the EDID blocks.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 48 ++
1 file changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 18b0ff223885..40ffd7aba157 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1600,6 +1600,54 @@ static const void *edid_extension_block_data(const
struct edid *edid, int index)
return edid_block_data(edid, index + 1);
}
+/*
+ * EDID base and extension block iterator.
+ *
+ * struct drm_edid_iter iter;
+ * const u8 *block;
+ *
+ * drm_edid_iter_begin(edid, &iter);
+ * drm_edid_iter_for_each(block, &iter) {
+ * // do stuff with block
+ * }
+ * drm_edid_iter_end(&iter);
+ */
+struct drm_edid_iter {
+ const struct edid *edid;
+
+ /* Current block index. */
+ int index;
+};
+
+static void drm_edid_iter_begin(const struct edid *edid,
+ struct drm_edid_iter *iter)
+{
+ memset(iter, 0, sizeof(*iter));
+
+ iter->edid = edid;
+}
+
+static const void *__drm_edid_iter_next(struct drm_edid_iter *iter)
+{
+ const void *block = NULL;
+
+ if (!iter->edid)
+ return NULL;
+
+ if (iter->index < edid_block_count(iter->edid))
+ block = edid_block_data(iter->edid, iter->index++);
+
+ return block;
+}
+
+#define drm_edid_iter_for_each(__block, __iter)\
+ while (((__block) = __drm_edid_iter_next(__iter)))
+
+static void drm_edid_iter_end(struct drm_edid_iter *iter)
+{
+ memset(iter, 0, sizeof(*iter));
+}
+
static const u8 edid_header[] = {
0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
};
--
2.30.2
[PATCH 08/19] drm/edid: convert drm_edid_to_speaker_allocation() to use cea db iter
Use the cea db iterator for speaker allocation. We'll still stop at the
first speaker data block, but not at the first CTA extension if that
doesn't have the info.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 47 --
1 file changed, 15 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index d02588637879..dac6a400905b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5093,42 +5093,25 @@ EXPORT_SYMBOL(drm_edid_to_sad);
*/
int drm_edid_to_speaker_allocation(const struct edid *edid, u8 **sadb)
{
+ const struct cea_db *db;
+ struct cea_db_iter iter;
int count = 0;
- int i, start, end, dbl;
- const u8 *cea;
- cea = drm_find_cea_extension(edid);
- if (!cea) {
- DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
- return 0;
- }
-
- if (cea_revision(cea) < 3) {
- DRM_DEBUG_KMS("SAD: wrong CEA revision\n");
- return 0;
- }
-
- if (cea_db_offsets(cea, &start, &end)) {
- DRM_DEBUG_KMS("SAD: invalid data block offsets\n");
- return -EPROTO;
- }
-
- for_each_cea_db(cea, i, start, end) {
- const u8 *db = &cea[i];
-
- if (cea_db_tag(db) == CTA_DB_SPEAKER) {
- dbl = cea_db_payload_len(db);
-
- /* Speaker Allocation Data Block */
- if (dbl == 3) {
- *sadb = kmemdup(&db[1], dbl, GFP_KERNEL);
- if (!*sadb)
- return -ENOMEM;
- count = dbl;
- break;
- }
+ cea_db_iter_edid_begin(edid, &iter);
+ cea_db_iter_for_each(db, &iter) {
+ if (cea_db_tag(db) == CTA_DB_SPEAKER &&
+ cea_db_payload_len(db) == 3) {
+ *sadb = kmemdup(db->data, cea_db_payload_len(db),
+ GFP_KERNEL);
+ if (!*sadb)
+ return -ENOMEM;
+ count = cea_db_payload_len(db);
+ break;
}
}
+ cea_db_iter_end(&iter);
+
+ DRM_DEBUG_KMS("Found %d Speaker Allocation Data Blocks\n", count);
return count;
}
--
2.30.2
[PATCH 07/19] drm/edid: convert add_cea_modes() to use cea db iter
Iterate through all CTA EDID extension blocks and DisplayID CTA data
blocks to add CEA modes.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 67 ++
1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 34897b1417a5..d02588637879 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4713,46 +4713,41 @@ static void drm_parse_y420cmdb_bitmap(struct
drm_connector *connector,
static int
add_cea_modes(struct drm_connector *connector, const struct edid *edid)
{
- const u8 *cea = drm_find_cea_extension(edid);
- const u8 *db, *hdmi = NULL, *video = NULL;
- u8 dbl, hdmi_len, video_len = 0;
+ const struct cea_db *db;
+ struct cea_db_iter iter;
int modes = 0;
- if (cea && cea_revision(cea) >= 3) {
- int i, start, end;
-
- if (cea_db_offsets(cea, &start, &end))
- return 0;
-
- for_each_cea_db(cea, i, start, end) {
- db = &cea[i];
- dbl = cea_db_payload_len(db);
-
- if (cea_db_tag(db) == CTA_DB_VIDEO) {
- video = db + 1;
- video_len = dbl;
- modes += do_cea_modes(connector, video, dbl);
- } else if (cea_db_is_hdmi_vsdb(db)) {
- hdmi = db;
- hdmi_len = dbl;
- } else if (cea_db_is_y420vdb(db)) {
- const u8 *vdb420 = &db[2];
-
- /* Add 4:2:0(only) modes present in EDID */
- modes += do_y420vdb_modes(connector,
- vdb420,
- dbl - 1);
- }
+ cea_db_iter_edid_begin(edid, &iter);
+ cea_db_iter_for_each(db, &iter) {
+ const u8 *hdmi = NULL, *video = NULL;
+ u8 hdmi_len = 0, video_len = 0;
+
+ if (cea_db_tag(db) == CTA_DB_VIDEO) {
+ video = cea_db_data(db);
+ video_len = cea_db_payload_len(db);
+ modes += do_cea_modes(connector, video, video_len);
+ } else if (cea_db_is_hdmi_vsdb(db)) {
+ /* FIXME: Switch to use cea_db_data() */
+ hdmi = (const u8 *)db;
+ hdmi_len = cea_db_payload_len(db);
+ } else if (cea_db_is_y420vdb(db)) {
+ const u8 *vdb420 = cea_db_data(db) + 1;
+
+ /* Add 4:2:0(only) modes present in EDID */
+ modes += do_y420vdb_modes(connector, vdb420,
+ cea_db_payload_len(db) - 1);
}
- }
- /*
-* We parse the HDMI VSDB after having added the cea modes as we will
-* be patching their flags when the sink supports stereo 3D.
-*/
- if (hdmi)
- modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, video,
- video_len);
+ /*
+* We parse the HDMI VSDB after having added the cea modes as we
+* will be patching their flags when the sink supports stereo
+* 3D.
+*/
+ if (hdmi)
+ modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len,
+ video, video_len);
+ }
+ cea_db_iter_end(&iter);
return modes;
}
--
2.30.2
[PATCH 10/19] drm/edid: convert drm_detect_hdmi_monitor() to use cea db iter
Iterate through all CTA data blocks, not just the first CTA extension.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 164a2020f9e1..b1fac281fd85 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5155,27 +5155,24 @@ EXPORT_SYMBOL(drm_av_sync_delay);
*/
bool drm_detect_hdmi_monitor(const struct edid *edid)
{
- const u8 *edid_ext;
- int i;
- int start_offset, end_offset;
-
- edid_ext = drm_find_cea_extension(edid);
- if (!edid_ext)
- return false;
-
- if (cea_db_offsets(edid_ext, &start_offset, &end_offset))
- return false;
+ const struct cea_db *db;
+ struct cea_db_iter iter;
+ bool hdmi = false;
/*
* Because HDMI identifier is in Vendor Specific Block,
* search it from all data blocks of CEA extension.
*/
- for_each_cea_db(edid_ext, i, start_offset, end_offset) {
- if (cea_db_is_hdmi_vsdb(&edid_ext[i]))
- return true;
+ cea_db_iter_edid_begin(edid, &iter);
+ cea_db_iter_for_each(db, &iter) {
+ if (cea_db_is_hdmi_vsdb(db)) {
+ hdmi = true;
+ break;
+ }
}
+ cea_db_iter_end(&iter);
- return false;
+ return hdmi;
}
EXPORT_SYMBOL(drm_detect_hdmi_monitor);
--
2.30.2
[PATCH 11/19] drm/edid: convert drm_detect_monitor_audio() to use cea db iter
Iterate through all CEA data blocks.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b1fac281fd85..e47bbcd103e6 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5190,10 +5190,10 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor);
*/
bool drm_detect_monitor_audio(const struct edid *edid)
{
+ const struct cea_db *db;
+ struct cea_db_iter iter;
const u8 *edid_ext;
- int i, j;
bool has_audio = false;
- int start_offset, end_offset;
edid_ext = drm_find_cea_extension(edid);
if (!edid_ext)
@@ -5207,18 +5207,21 @@ bool drm_detect_monitor_audio(const struct edid *edid)
goto end;
}
- if (cea_db_offsets(edid_ext, &start_offset, &end_offset))
- goto end;
+ cea_db_iter_edid_begin(edid, &iter);
+ cea_db_iter_for_each(db, &iter) {
+ if (cea_db_tag(db) == CTA_DB_AUDIO) {
+ const u8 *data = cea_db_data(db);
+ int i;
- for_each_cea_db(edid_ext, i, start_offset, end_offset) {
- if (cea_db_tag(&edid_ext[i]) == CTA_DB_AUDIO) {
- has_audio = true;
- for (j = 1; j < cea_db_payload_len(&edid_ext[i]) + 1; j
+= 3)
+ for (i = 0; i < cea_db_payload_len(db); i += 3)
DRM_DEBUG_KMS("CEA audio format %d\n",
- (edid_ext[i + j] >> 3) & 0xf);
- goto end;
+ (data[i] >> 3) & 0xf);
+ has_audio = true;
+ break;
}
}
+ cea_db_iter_end(&iter);
+
end:
return has_audio;
}
--
2.30.2
Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Use i915_probe_error instead of drm_err
On 4/13/2022 11:41 PM, Anshuman Gupta wrote:
On 2022-04-13 at 04:18:52 +0530, Vinay Belgaumkar wrote:
This will ensure we don't have false positives when we run
error injection tests.
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 42 ++---
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index b170238aa15c..639de3c10545 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -152,8 +152,8 @@ static int slpc_query_task_state(struct intel_guc_slpc
*slpc)
ret = guc_action_slpc_query(guc, offset);
if (unlikely(ret))
As commit logs describe, this code patch can hit, when we run error injection
test.
Do we need unlikely() here?
Br,
Anshuman Gupta.
I think we still need the unlikely(). Majority of the time, we still
need the compiler optimization.
Only in the rare case of running the error injection test will it not be
needed.
Thanks,
Vinay.
- drm_err(&i915->drm, "Failed to query task state (%pe)\n",
- ERR_PTR(ret));
+ i915_probe_error(i915, "Failed to query task state (%pe)\n",
+ERR_PTR(ret));
drm_clflush_virt_range(slpc->vaddr, SLPC_PAGE_SIZE_BYTES);
@@ -170,8 +170,8 @@ static int slpc_set_param(struct intel_guc_slpc *slpc, u8 id, u32 value)
ret = guc_action_slpc_set_param(guc, id, value);
if (ret)
- drm_err(&i915->drm, "Failed to set param %d to %u (%pe)\n",
- id, value, ERR_PTR(ret));
+ i915_probe_error(i915, "Failed to set param %d to %u (%pe)\n",
+id, value, ERR_PTR(ret));
return ret;
}
@@ -211,8 +211,8 @@ static int slpc_force_min_freq(struct intel_guc_slpc *slpc,
u32 freq)
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
freq);
if (ret)
- drm_err(&i915->drm, "Unable to force min freq to %u:
%d",
- freq, ret);
+ i915_probe_error(i915, "Unable to force min freq to %u:
%d",
+freq, ret);
}
return ret;
@@ -247,9 +247,9 @@ int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma, (void **)&slpc->vaddr);
if (unlikely(err)) {
- drm_err(&i915->drm,
- "Failed to allocate SLPC struct (err=%pe)\n",
- ERR_PTR(err));
+ i915_probe_error(i915,
+"Failed to allocate SLPC struct (err=%pe)\n",
+ERR_PTR(err));
return err;
}
@@ -316,15 +316,15 @@ static int slpc_reset(struct intel_guc_slpc *slpc)
ret = guc_action_slpc_reset(guc, offset);
if (unlikely(ret < 0)) {
- drm_err(&i915->drm, "SLPC reset action failed (%pe)\n",
- ERR_PTR(ret));
+ i915_probe_error(i915, "SLPC reset action failed (%pe)\n",
+ERR_PTR(ret));
return ret;
}
if (!ret) {
if (wait_for(slpc_is_running(slpc), SLPC_RESET_TIMEOUT_MS)) {
- drm_err(&i915->drm, "SLPC not enabled! State = %s\n",
- slpc_get_state_string(slpc));
+ i915_probe_error(i915, "SLPC not enabled! State = %s\n",
+slpc_get_state_string(slpc));
return -EIO;
}
}
@@ -616,8 +616,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
ret = slpc_reset(slpc);
if (unlikely(ret < 0)) {
- drm_err(&i915->drm, "SLPC Reset event returned (%pe)\n",
- ERR_PTR(ret));
+ i915_probe_error(i915, "SLPC Reset event returned (%pe)\n",
+ERR_PTR(ret));
return ret;
}
@@ -632,24 +632,24 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
/* Ignore efficient freq and set min to platform min */
ret = slpc_ignore_eff_freq(slpc, true);
if (unlikely(ret)) {
- drm_err(&i915->drm, "Failed to set SLPC min to RPn (%pe)\n",
- ERR_PTR(ret));
+ i915_probe_error(i915, "Failed to set SLPC min to RPn (%pe)\n",
+ERR_PTR(ret));
return ret;
}
/* Set SLPC max limit to RP0 */
ret = slpc_use_fused_rp0(slpc);
if (unlikely(ret)) {
- drm_err(&i915->drm, "Failed to set SLPC max to RP0 (%pe)\n",
- ERR_PTR(ret));
+ i915_probe_error(i915,
[PATCH 13/19] drm/edid: convert drm_edid_to_eld() to use cea db iter
Iterate through all CTA data blocks across all CTA extensions and
DisplayID data blocks. This may gather more data than before, and if
there's duplicated data, some is overwritten by whichever comes last.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 64 +-
1 file changed, 29 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 91f15b561d78..fa7f2bf68da3 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4935,12 +4935,12 @@ static void clear_eld(struct drm_connector *connector)
static void drm_edid_to_eld(struct drm_connector *connector,
const struct edid *edid)
{
+ const struct cea_db *db;
+ struct cea_db_iter iter;
uint8_t *eld = connector->eld;
const u8 *cea;
- const u8 *db;
int total_sad_count = 0;
int mnl;
- int dbl;
clear_eld(connector);
@@ -4966,43 +4966,37 @@ static void drm_edid_to_eld(struct drm_connector
*connector,
eld[DRM_ELD_PRODUCT_CODE0] = edid->prod_code[0];
eld[DRM_ELD_PRODUCT_CODE1] = edid->prod_code[1];
- if (cea_revision(cea) >= 3) {
- int i, start, end;
+ cea_db_iter_edid_begin(edid, &iter);
+ cea_db_iter_for_each(db, &iter) {
+ const u8 *data = cea_db_data(db);
+ int len = cea_db_payload_len(db);
int sad_count;
- if (cea_db_offsets(cea, &start, &end)) {
- start = 0;
- end = 0;
- }
-
- for_each_cea_db(cea, i, start, end) {
- db = &cea[i];
- dbl = cea_db_payload_len(db);
-
- switch (cea_db_tag(db)) {
- case CTA_DB_AUDIO:
- /* Audio Data Block, contains SADs */
- sad_count = min(dbl / 3, 15 - total_sad_count);
- if (sad_count >= 1)
- memcpy(&eld[DRM_ELD_CEA_SAD(mnl,
total_sad_count)],
- &db[1], sad_count * 3);
- total_sad_count += sad_count;
- break;
- case CTA_DB_SPEAKER:
- /* Speaker Allocation Data Block */
- if (dbl >= 1)
- eld[DRM_ELD_SPEAKER] = db[1];
- break;
- case CTA_DB_VENDOR:
- /* HDMI Vendor-Specific Data Block */
- if (cea_db_is_hdmi_vsdb(db))
- drm_parse_hdmi_vsdb_audio(connector,
db);
- break;
- default:
- break;
- }
+ switch (cea_db_tag(db)) {
+ case CTA_DB_AUDIO:
+ /* Audio Data Block, contains SADs */
+ sad_count = min(len / 3, 15 - total_sad_count);
+ if (sad_count >= 1)
+ memcpy(&eld[DRM_ELD_CEA_SAD(mnl,
total_sad_count)],
+ data, sad_count * 3);
+ total_sad_count += sad_count;
+ break;
+ case CTA_DB_SPEAKER:
+ /* Speaker Allocation Data Block */
+ if (len >= 1)
+ eld[DRM_ELD_SPEAKER] = data[0];
+ break;
+ case CTA_DB_VENDOR:
+ /* HDMI Vendor-Specific Data Block */
+ if (cea_db_is_hdmi_vsdb(db))
+ drm_parse_hdmi_vsdb_audio(connector, (const u8
*)db);
+ break;
+ default:
+ break;
}
}
+ cea_db_iter_end(&iter);
+
eld[DRM_ELD_SAD_COUNT_CONN_TYPE] |= total_sad_count <<
DRM_ELD_SAD_COUNT_SHIFT;
if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
--
2.30.2
[PATCH 09/19] drm/edid: convert drm_edid_to_sad() to use cea db iter
Use the cea db iterator for short audio descriptors. We'll still stop at
the first audio data block, but not at the first CTA Extension if that
doesn't have the info.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 34 +-
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dac6a400905b..164a2020f9e1 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5031,40 +5031,21 @@ static void drm_edid_to_eld(struct drm_connector
*connector,
*/
int drm_edid_to_sad(const struct edid *edid, struct cea_sad **sads)
{
+ const struct cea_db *db;
+ struct cea_db_iter iter;
int count = 0;
- int i, start, end, dbl;
- const u8 *cea;
-
- cea = drm_find_cea_extension(edid);
- if (!cea) {
- DRM_DEBUG_KMS("SAD: no CEA Extension found\n");
- return 0;
- }
-
- if (cea_revision(cea) < 3) {
- DRM_DEBUG_KMS("SAD: wrong CEA revision\n");
- return 0;
- }
-
- if (cea_db_offsets(cea, &start, &end)) {
- DRM_DEBUG_KMS("SAD: invalid data block offsets\n");
- return -EPROTO;
- }
-
- for_each_cea_db(cea, i, start, end) {
- const u8 *db = &cea[i];
+ cea_db_iter_edid_begin(edid, &iter);
+ cea_db_iter_for_each(db, &iter) {
if (cea_db_tag(db) == CTA_DB_AUDIO) {
int j;
- dbl = cea_db_payload_len(db);
-
- count = dbl / 3; /* SAD is 3B */
+ count = cea_db_payload_len(db) / 3; /* SAD is 3B */
*sads = kcalloc(count, sizeof(**sads), GFP_KERNEL);
if (!*sads)
return -ENOMEM;
for (j = 0; j < count; j++) {
- const u8 *sad = &db[1 + j * 3];
+ const u8 *sad = &db->data[j * 3];
(*sads)[j].format = (sad[0] & 0x78) >> 3;
(*sads)[j].channels = sad[0] & 0x7;
@@ -5074,6 +5055,9 @@ int drm_edid_to_sad(const struct edid *edid, struct
cea_sad **sads)
break;
}
}
+ cea_db_iter_end(&iter);
+
+ DRM_DEBUG_KMS("Found %d Short Audio Descriptors\n", count);
return count;
}
--
2.30.2
[PATCH 14/19] drm/edid: sunset the old unused cea data block iterators
All CTA data block iteration has now been converted to the new cea db
iterators.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 45 --
1 file changed, 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fa7f2bf68da3..7ed0258c0edf 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4373,48 +4373,6 @@ cea_revision(const u8 *cea)
return cea[1];
}
-static int
-cea_db_offsets(const u8 *cea, int *start, int *end)
-{
- /* DisplayID CTA extension blocks and top-level CEA EDID
-* block header definitions differ in the following bytes:
-* 1) Byte 2 of the header specifies length differently,
-* 2) Byte 3 is only present in the CEA top level block.
-*
-* The different definitions for byte 2 follow.
-*
-* DisplayID CTA extension block defines byte 2 as:
-* Number of payload bytes
-*
-* CEA EDID block defines byte 2 as:
-* Byte number (decimal) within this block where the 18-byte
-* DTDs begin. If no non-DTD data is present in this extension
-* block, the value should be set to 04h (the byte after next).
-* If set to 00h, there are no DTDs present in this block and
-* no non-DTD data.
-*/
- if (cea[0] == DATA_BLOCK_CTA) {
- /*
-* for_each_displayid_db() has already verified
-* that these stay within expected bounds.
-*/
- *start = 3;
- *end = *start + cea[2];
- } else if (cea[0] == CEA_EXT) {
- /* Data block offset in CEA extension block */
- *start = 4;
- *end = cea[2];
- if (*end == 0)
- *end = 127;
- if (*end < 4 || *end > 127)
- return -ERANGE;
- } else {
- return -EOPNOTSUPP;
- }
-
- return 0;
-}
-
/*
* CTA Data Block iterator.
*
@@ -4667,9 +4625,6 @@ static bool cea_db_is_hdmi_hdr_metadata_block(const void
*db)
cea_db_payload_len(db) >= 3;
}
-#define for_each_cea_db(cea, i, start, end) \
- for ((i) = (start); (i) < (end) && (i) +
cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)])
+ 1)
-
static void drm_parse_y420cmdb_bitmap(struct drm_connector *connector,
const u8 *db)
{
--
2.30.2
[PATCH 15/19] drm/edid: restore some type safety to cea_db_*() functions
During the transition, we accepted a void pointer for a poor C
programmer's version of polymorphism. Switch the functions to use struct
cea_db * to regain some more type safety.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 7ed0258c0edf..1fc4e8754cd4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4408,11 +4408,8 @@ struct cea_db {
u8 data[];
} __packed;
-static int cea_db_tag(const void *_db)
+static int cea_db_tag(const struct cea_db *db)
{
- /* FIXME: Transition to passing struct cea_db * everywhere. */
- const struct cea_db *db = _db;
-
return db->tag_length >> 5;
}
@@ -4579,47 +4576,47 @@ static void cea_db_iter_end(struct cea_db_iter *iter)
memset(iter, 0, sizeof(*iter));
}
-static bool cea_db_is_hdmi_vsdb(const void *db)
+static bool cea_db_is_hdmi_vsdb(const struct cea_db *db)
{
return cea_db_is_vendor(db, HDMI_IEEE_OUI) &&
cea_db_payload_len(db) >= 5;
}
-static bool cea_db_is_hdmi_forum_vsdb(const void *db)
+static bool cea_db_is_hdmi_forum_vsdb(const struct cea_db *db)
{
return cea_db_is_vendor(db, HDMI_FORUM_IEEE_OUI) &&
cea_db_payload_len(db) >= 7;
}
-static bool cea_db_is_microsoft_vsdb(const void *db)
+static bool cea_db_is_microsoft_vsdb(const struct cea_db *db)
{
return cea_db_is_vendor(db, MICROSOFT_IEEE_OUI) &&
cea_db_payload_len(db) == 21;
}
-static bool cea_db_is_vcdb(const void *db)
+static bool cea_db_is_vcdb(const struct cea_db *db)
{
return cea_db_is_extended_tag(db, CTA_EXT_DB_VIDEO_CAP) &&
cea_db_payload_len(db) == 2;
}
-static bool cea_db_is_hdmi_forum_scdb(const void *db)
+static bool cea_db_is_hdmi_forum_scdb(const struct cea_db *db)
{
return cea_db_is_extended_tag(db, CTA_EXT_DB_HF_SCDB) &&
cea_db_payload_len(db) >= 7;
}
-static bool cea_db_is_y420cmdb(const void *db)
+static bool cea_db_is_y420cmdb(const struct cea_db *db)
{
return cea_db_is_extended_tag(db, CTA_EXT_DB_420_VIDEO_CAP_MAP);
}
-static bool cea_db_is_y420vdb(const void *db)
+static bool cea_db_is_y420vdb(const struct cea_db *db)
{
return cea_db_is_extended_tag(db, CTA_EXT_DB_420_VIDEO_DATA);
}
-static bool cea_db_is_hdmi_hdr_metadata_block(const void *db)
+static bool cea_db_is_hdmi_hdr_metadata_block(const struct cea_db *db)
{
return cea_db_is_extended_tag(db, CTA_EXT_DB_HDR_STATIC_METADATA) &&
cea_db_payload_len(db) >= 3;
--
2.30.2
[PATCH 12/19] drm/edid: convert drm_parse_cea_ext() to use cea db iter
Iterate through all CTA data blocks across all CTA Extensions and
DisplayID data blocks.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e47bbcd103e6..91f15b561d78 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5520,8 +5520,9 @@ static void drm_parse_cea_ext(struct drm_connector
*connector,
const struct edid *edid)
{
struct drm_display_info *info = &connector->display_info;
+ const struct cea_db *db;
+ struct cea_db_iter iter;
const u8 *edid_ext;
- int i, start, end;
edid_ext = drm_find_cea_extension(edid);
if (!edid_ext)
@@ -5540,26 +5541,26 @@ static void drm_parse_cea_ext(struct drm_connector
*connector,
info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
}
- if (cea_db_offsets(edid_ext, &start, &end))
- return;
-
- for_each_cea_db(edid_ext, i, start, end) {
- const u8 *db = &edid_ext[i];
+ cea_db_iter_edid_begin(edid, &iter);
+ cea_db_iter_for_each(db, &iter) {
+ /* FIXME: convert parsers to use struct cea_db */
+ const u8 *data = (const u8 *)db;
if (cea_db_is_hdmi_vsdb(db))
- drm_parse_hdmi_vsdb_video(connector, db);
+ drm_parse_hdmi_vsdb_video(connector, data);
if (cea_db_is_hdmi_forum_vsdb(db) ||
cea_db_is_hdmi_forum_scdb(db))
- drm_parse_hdmi_forum_vsdb(connector, db);
+ drm_parse_hdmi_forum_vsdb(connector, data);
if (cea_db_is_microsoft_vsdb(db))
- drm_parse_microsoft_vsdb(connector, db);
+ drm_parse_microsoft_vsdb(connector, data);
if (cea_db_is_y420cmdb(db))
- drm_parse_y420cmdb_bitmap(connector, db);
+ drm_parse_y420cmdb_bitmap(connector, data);
if (cea_db_is_vcdb(db))
- drm_parse_vcdb(connector, db);
+ drm_parse_vcdb(connector, data);
if (cea_db_is_hdmi_hdr_metadata_block(db))
- drm_parse_hdr_metadata_block(connector, db);
+ drm_parse_hdr_metadata_block(connector, data);
}
+ cea_db_iter_end(&iter);
}
static
--
2.30.2
[PATCH 16/19] drm/edid: detect basic audio in all CEA extensions
Convert drm_find_cea_extension() to EDID block iterator in basic audio
detection. Detect basic audio in all CEA extensions.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1fc4e8754cd4..091f68102f77 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5136,17 +5136,21 @@ EXPORT_SYMBOL(drm_detect_hdmi_monitor);
*/
bool drm_detect_monitor_audio(const struct edid *edid)
{
+ struct drm_edid_iter edid_iter;
const struct cea_db *db;
struct cea_db_iter iter;
const u8 *edid_ext;
bool has_audio = false;
- edid_ext = drm_find_cea_extension(edid);
- if (!edid_ext)
- goto end;
-
- has_audio = (edid_ext[0] == CEA_EXT &&
- (edid_ext[3] & EDID_BASIC_AUDIO) != 0);
+ drm_edid_iter_begin(edid, &edid_iter);
+ drm_edid_iter_for_each(edid_ext, &edid_iter) {
+ if (edid_ext[0] == CEA_EXT) {
+ has_audio = edid_ext[3] & EDID_BASIC_AUDIO;
+ if (has_audio)
+ break;
+ }
+ }
+ drm_edid_iter_end(&edid_iter);
if (has_audio) {
DRM_DEBUG_KMS("Monitor has basic audio support\n");
--
2.30.2
[PATCH 17/19] drm/edid: detect color formats and CTA revision in all CTA extensions
Convert drm_find_cea_extension() to EDID block iterator in color format
and CTA revision detection. Detect them in all CTA extensions.
Also parse CTA Data Blocks in DisplayID even if there's no CTA EDID
extension.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 091f68102f77..fad9fd13937b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5470,32 +5470,40 @@ static void drm_parse_cea_ext(struct drm_connector
*connector,
const struct edid *edid)
{
struct drm_display_info *info = &connector->display_info;
+ struct drm_edid_iter edid_iter;
const struct cea_db *db;
struct cea_db_iter iter;
const u8 *edid_ext;
- edid_ext = drm_find_cea_extension(edid);
- if (!edid_ext)
- return;
+ drm_edid_iter_begin(edid, &edid_iter);
+ drm_edid_iter_for_each(edid_ext, &edid_iter) {
+ if (edid_ext[0] != CEA_EXT)
+ continue;
- info->cea_rev = edid_ext[1];
+ if (!info->cea_rev)
+ info->cea_rev = edid_ext[1];
- /* The existence of a CEA block should imply RGB support */
- info->color_formats = DRM_COLOR_FORMAT_RGB444;
+ if (info->cea_rev != edid_ext[1])
+ DRM_DEBUG_KMS("CEA extension version mismatch %u !=
%u\n",
+ info->cea_rev, edid_ext[1]);
- /* CTA DisplayID Data Block does not have byte #3 */
- if (edid_ext[0] == CEA_EXT) {
+ /* The existence of a CTA extension should imply RGB support */
+ info->color_formats = DRM_COLOR_FORMAT_RGB444;
if (edid_ext[3] & EDID_CEA_YCRCB444)
info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
if (edid_ext[3] & EDID_CEA_YCRCB422)
info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
}
+ drm_edid_iter_end(&edid_iter);
cea_db_iter_edid_begin(edid, &iter);
cea_db_iter_for_each(db, &iter) {
/* FIXME: convert parsers to use struct cea_db */
const u8 *data = (const u8 *)db;
+ /* The existence of a CTA block should imply RGB support */
+ info->color_formats = DRM_COLOR_FORMAT_RGB444;
+
if (cea_db_is_hdmi_vsdb(db))
drm_parse_hdmi_vsdb_video(connector, data);
if (cea_db_is_hdmi_forum_vsdb(db) ||
--
2.30.2
[PATCH 18/19] drm/edid: skip CTA extension scan in drm_edid_to_eld() just for CTA rev
The DisplayID CTA data block version does not necessarily match the CTA
revision. Simplify by postponing drm_edid_to_eld() slightly, and reusing
the CTA revision extracted by drm_parse_cea_ext().
By not bailing out early in drm_edid_to_eld() we may end up filling
meaningless data to the ELD. However, the main decision for audio is not
the ELD, but rather drm_detect_monitor_audio() called by drivers.
(Arguably a future cleanup could do that in drm_add_edid_modes() and
cache the result in the connector.)
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fad9fd13937b..ccd7d075eeb8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4887,10 +4887,10 @@ static void clear_eld(struct drm_connector *connector)
static void drm_edid_to_eld(struct drm_connector *connector,
const struct edid *edid)
{
+ const struct drm_display_info *info = &connector->display_info;
const struct cea_db *db;
struct cea_db_iter iter;
uint8_t *eld = connector->eld;
- const u8 *cea;
int total_sad_count = 0;
int mnl;
@@ -4899,16 +4899,10 @@ static void drm_edid_to_eld(struct drm_connector
*connector,
if (!edid)
return;
- cea = drm_find_cea_extension(edid);
- if (!cea) {
- DRM_DEBUG_KMS("ELD: no CEA Extension found\n");
- return;
- }
-
mnl = get_monitor_name(edid, &eld[DRM_ELD_MONITOR_NAME_STRING]);
DRM_DEBUG_KMS("ELD monitor %s\n", &eld[DRM_ELD_MONITOR_NAME_STRING]);
- eld[DRM_ELD_CEA_EDID_VER_MNL] = cea[1] << DRM_ELD_CEA_EDID_VER_SHIFT;
+ eld[DRM_ELD_CEA_EDID_VER_MNL] = info->cea_rev <<
DRM_ELD_CEA_EDID_VER_SHIFT;
eld[DRM_ELD_CEA_EDID_VER_MNL] |= mnl;
eld[DRM_ELD_VER] = DRM_ELD_VER_CEA861D;
@@ -5847,8 +5841,6 @@ static int drm_edid_connector_update(struct drm_connector
*connector,
return 0;
}
- drm_edid_to_eld(connector, edid);
-
/*
* CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks.
* To avoid multiple parsing of same block, lets parse that map
@@ -5856,6 +5848,9 @@ static int drm_edid_connector_update(struct drm_connector
*connector,
*/
quirks = drm_add_display_info(connector, edid);
+ /* Depends on info->cea_rev set by drm_add_display_info() above */
+ drm_edid_to_eld(connector, edid);
+
/*
* EDID spec says modes should be preferred in this order:
* - preferred detailed mode
--
2.30.2
[PATCH 19/19] drm/edid: sunset drm_find_cea_extension()
Convert drm_find_cea_extension() to a predicate function to check if the
EDID has a CTA extension or a DisplayID CTA data block. This is mainly
to avoid adding new users that only find the first CTA extension.
Cc: Ville Syrjälä
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ccd7d075eeb8..4eeea4212f3a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3582,30 +3582,29 @@ const u8 *drm_find_edid_extension(const struct edid
*edid,
return edid_ext;
}
-static const u8 *drm_find_cea_extension(const struct edid *edid)
+/* Return true if the EDID has a CTA extension or a DisplayID CTA data block */
+static bool drm_edid_has_cta_extension(const struct edid *edid)
{
const struct displayid_block *block;
struct displayid_iter iter;
- const u8 *cea;
int ext_index = 0;
+ bool found = false;
/* Look for a top level CEA extension block */
- /* FIXME: make callers iterate through multiple CEA ext blocks? */
- cea = drm_find_edid_extension(edid, CEA_EXT, &ext_index);
- if (cea)
- return cea;
+ if (drm_find_edid_extension(edid, CEA_EXT, &ext_index))
+ return true;
/* CEA blocks can also be found embedded in a DisplayID block */
displayid_iter_edid_begin(edid, &iter);
displayid_iter_for_each(block, &iter) {
if (block->tag == DATA_BLOCK_CTA) {
- cea = (const u8 *)block;
+ found = true;
break;
}
}
displayid_iter_end(&iter);
- return cea;
+ return found;
}
static __always_inline const struct drm_display_mode *cea_mode_for_vic(u8 vic)
@@ -3878,8 +3877,8 @@ add_alternate_cea_modes(struct drm_connector *connector,
const struct edid *edid
LIST_HEAD(list);
int modes = 0;
- /* Don't add CEA modes if the CEA extension block is missing */
- if (!drm_find_cea_extension(edid))
+ /* Don't add CTA modes if the CTA extension block is missing */
+ if (!drm_edid_has_cta_extension(edid))
return 0;
/*
--
2.30.2
Re: [PATCH] drm/radeon: Add build directory to include path
Hi. On Thu, Apr 14, 2022 at 10:50 PM Michel Dänzer wrote: > > On 2022-04-14 15:34, Alex Deucher wrote: > > On Thu, Apr 14, 2022 at 4:44 AM Christian König > > wrote: > >> Am 14.04.22 um 09:37 schrieb Michel Dänzer: > >>> On 2022-04-14 08:24, Christian König wrote: > Am 13.04.22 um 18:14 schrieb Michel Dänzer: > > From: Michel Dänzer > > > > Fixes compile errors with out-of-tree builds, e.g. > > > > ../drivers/gpu/drm/radeon/r420.c:38:10: fatal error: r420_reg_safe.h: > > No such file or directory > > 38 | #include "r420_reg_safe.h" > > | ^ > > Well stuff like that usually points to a broken build environment. > >>> Just a separate build directory. Specifically, I'm hitting the errors with > >>> > >>> make -C build-amd64 M=drivers/gpu/drm Maybe make O=build-arm64 drivers/gpu/drm/ is the way you were searching for. It builds only drivers/gpu/drm/ in the separate directory. > >>> > >>> Generated headers such as r420_reg_safe.h reside in the build directory, > >>> so source files in the source directory can't find them without an > >>> explicit search path. > >> > >> I'm trying to swap back into my brain how all of this used to work, but > >> that's a really long time ago that I tried this as well. > >> > >>> Are you saying that should get added automagically somehow? For the kernel tree, yes, it is done automatically. See the code in scripts/Makefile.lib: # $(srctree)/$(src) for including checkin headers from generated source files # $(objtree)/$(obj) for including generated headers from checkin source files ifeq ($(KBUILD_EXTMOD),) ifdef building_out_of_srctree _c_flags += -I $(srctree)/$(src) -I $(objtree)/$(obj) _a_flags += -I $(srctree)/$(src) -I $(objtree)/$(obj) _cpp_flags += -I $(srctree)/$(src) -I $(objtree)/$(obj) endif endif But, you used M=drivers/gpu/drm. So, it did not work. M= is intended for building external modules. I do not recommend it for in-tree drivers. > >> > >> Yes, exactly that. I'm like 95% sure that used to work, but I don't know > >> why exactly either. > >> > >>> FWIW, this is pretty common in the kernel according to git grep. > >> > >> Maybe Alex or somebody else with some more background in the kernel > >> Makefiles could jump in and help here. > > > > I don't remember either. I vaguely recall the build support for the > > mkregtable stuff being reworked a while ago. A quick zip through the > > git logs shows a series from Masahiro Yamada from 2020. > > Yamada-san, can you help us? :) > > See https://patchwork.freedesktop.org/patch/482011/ for my patch. > > > -- > Earthling Michel Dänzer| https://redhat.com > Libre software enthusiast | Mesa and Xwayland developer -- Best Regards Masahiro Yamada
Re: [PATCH v18 07/10] drm/mediatek: add mediatek-drm of vdosys0 support for mt8195
Hi "jason-jh.lin",
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next]
[also build test WARNING on robh/for-next krzk/for-next linus/master v5.18-rc2
next-20220414]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/intel-lab-lkp/linux/commits/jason-jh-lin/Add-Mediatek-Soc-DRM-vdosys0-support-for-mt8195/20220412-183359
base: git://anongit.freedesktop.org/drm/drm drm-next
config: arm64-buildonly-randconfig-r001-20220413
(https://download.01.org/0day-ci/archive/20220414/202204142333.qxgcgmi1-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project
6b7e6ea489f6dd45a9b0da9ac20871560917b9b0)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
#
https://github.com/intel-lab-lkp/linux/commit/7c175317aa80bbc885609a730214448147a46b47
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
jason-jh-lin/Add-Mediatek-Soc-DRM-vdosys0-support-for-mt8195/20220412-183359
git checkout 7c175317aa80bbc885609a730214448147a46b47
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/gpu/drm/mediatek/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/gpu/drm/mediatek/mtk_drm_drv.c:707:15: warning: cast to smaller
integer type 'enum mtk_ddp_comp_type' from 'const void *'
[-Wvoid-pointer-to-enum-cast]
comp_type = (enum mtk_ddp_comp_type)of_id->data;
^~~
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c:203:42: warning: unused variable
>> 'mt2701_mmsys_match_data' [-Wunused-const-variable]
static const struct mtk_mmsys_match_data mt2701_mmsys_match_data = {
^
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c:218:42: warning: unused variable
>> 'mt7623_mmsys_match_data' [-Wunused-const-variable]
static const struct mtk_mmsys_match_data mt7623_mmsys_match_data = {
^
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c:234:42: warning: unused variable
>> 'mt2712_mmsys_match_data' [-Wunused-const-variable]
static const struct mtk_mmsys_match_data mt2712_mmsys_match_data = {
^
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c:246:42: warning: unused variable
>> 'mt8167_mmsys_match_data' [-Wunused-const-variable]
static const struct mtk_mmsys_match_data mt8167_mmsys_match_data = {
^
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c:260:42: warning: unused variable
>> 'mt8173_mmsys_match_data' [-Wunused-const-variable]
static const struct mtk_mmsys_match_data mt8173_mmsys_match_data = {
^
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c:274:42: warning: unused variable
>> 'mt8183_mmsys_match_data' [-Wunused-const-variable]
static const struct mtk_mmsys_match_data mt8183_mmsys_match_data = {
^
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c:288:42: warning: unused variable
>> 'mt8192_mmsys_match_data' [-Wunused-const-variable]
static const struct mtk_mmsys_match_data mt8192_mmsys_match_data = {
^
>> drivers/gpu/drm/mediatek/mtk_drm_drv.c:305:42: warning: unused variable
>> 'mt8195_mmsys_match_data' [-Wunused-const-variable]
static const struct mtk_mmsys_match_data mt8195_mmsys_match_data = {
^
9 warnings generated.
vim +/mt2701_mmsys_match_data +203 drivers/gpu/drm/mediatek/mtk_drm_drv.c
202
> 203 static const struct mtk_mmsys_match_data mt2701_mmsys_match_data = {
204 .num_drv_data = 1,
205 .drv_data = {
206 &mt2701_mmsys_driver_data,
207 },
208 };
209
210 static const struct mtk_mmsys_driver_data mt7623_mmsys_driver_data = {
211 .main_path = mt7623_mtk_ddp_main,
212 .main_len = ARRAY_SIZE(mt7623_mtk_ddp_main),
213 .ext_path = mt7623_mtk_ddp_ext,
214 .ext_len = ARRAY_SIZ
Re: [PATCH v5 03/10] drm/hdcp: Update property value on content type and user changes
On Tue, Apr 12, 2022 at 09:25:59AM -0400, Rodrigo Vivi wrote:
> On Mon, Apr 11, 2022 at 08:47:32PM +, Sean Paul wrote:
> > From: Sean Paul
> >
> > This patch updates the connector's property value in 2 cases which were
> > previously missed:
> >
> > 1- Content type changes. The value should revert back to DESIRED from
> >ENABLED in case the driver must re-authenticate the link due to the
> >new content type.
> >
> > 2- Userspace sets value to DESIRED while ENABLED. In this case, the
> >value should be reset immediately to ENABLED since the link is
> >actively being encrypted.
> >
> > To accommodate these changes, I've split up the conditionals to make
> > things a bit more clear (as much as one can with this mess of state).
> >
> > Acked-by: Jani Nikula
> > Reviewed-by: Abhinav Kumar
> > Signed-off-by: Sean Paul
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210913175747.47456-4-s...@poorly.run
> > #v1
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20210915203834.1439-4-s...@poorly.run
> > #v2
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20211001151145.55916-4-s...@poorly.run
> > #v3
> > Link:
> > https://patchwork.freedesktop.org/patch/msgid/20211105030434.2828845-4-s...@poorly.run
> > #v4
> >
> > Changes in v2:
> > -None
> > Changes in v3:
> > -Fixed indentation issue identified by 0-day
> > Changes in v4:
> > -None
> > Changes in v5:
> > -None
> > ---
> > drivers/gpu/drm/drm_hdcp.c | 26 +-
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_hdcp.c b/drivers/gpu/drm/drm_hdcp.c
> > index dd8fa91c51d6..8c851d40cd45 100644
> > --- a/drivers/gpu/drm/drm_hdcp.c
> > +++ b/drivers/gpu/drm/drm_hdcp.c
> > @@ -487,21 +487,29 @@ bool drm_hdcp_atomic_check(struct drm_connector
> > *connector,
> > return true;
> >
> > /*
> > -* Nothing to do if content type is unchanged and one of:
> > -* - state didn't change
> > +* Content type changes require an HDCP disable/enable cycle.
> > +*/
> > + if (new_conn_state->hdcp_content_type !=
> > old_conn_state->hdcp_content_type) {
>
> shouldn't we add some && ( old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) {
> here?
Thanks for your reviews Rodrigo.
I don't think so since the content type is changing the current state of old
content protection is immaterial (ie: if we need to enable HDCP 2.x, the state
of HDCP 1.x doesn't really matter), we need to re-evaluate whether the current
level of HDCP is sufficient.
Hopefully that makes sense, but I could be missing something :-)
Sean
>
> > + new_conn_state->content_protection =
> > + DRM_MODE_CONTENT_PROTECTION_DESIRED;
> > + return true;
> > + }
> > +
> > + /*
> > +* Ignore meaningless state changes:
> > * - HDCP was activated since the last commit
> > -* - attempting to set to desired while already enabled
> > +* - Attempting to set to desired while already enabled
> > */
> > - if (old_hdcp == new_hdcp ||
> > - (old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> > + if ((old_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED &&
> > new_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED) ||
> > (old_hdcp == DRM_MODE_CONTENT_PROTECTION_ENABLED &&
> > new_hdcp == DRM_MODE_CONTENT_PROTECTION_DESIRED)) {
> > - if (old_conn_state->hdcp_content_type ==
> > - new_conn_state->hdcp_content_type)
> > - return false;
> > + new_conn_state->content_protection =
> > + DRM_MODE_CONTENT_PROTECTION_ENABLED;
> > + return false;
> > }
> >
> > - return true;
> > + /* Finally, if state changes, we need action */
> > + return old_hdcp != new_hdcp;
> > }
> > EXPORT_SYMBOL(drm_hdcp_atomic_check);
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> >
--
Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH v5 00/10] drm/hdcp: Pull HDCP auth/exchange/check into helpers
On Tue, Apr 12, 2022 at 09:41:35AM -0400, Rodrigo Vivi wrote: > On Mon, Apr 11, 2022 at 08:47:29PM +, Sean Paul wrote: > > From: Sean Paul > > > > Rebased set from November. Fixed a nit from Stephen in the msm patch and > > moved hdcp registers into the trogdor dtsi file to avoid differences > > with sc7180-based windows devices. The set is 4 patches lighter since > > some of the changes were accepted into msm. > > > > I'm still waiting for Intel review of the first 7 patches. Rodrigo/Jani, > > would you please provide your input so we can move forward with this > > set? > > I'm a bit concerned with patches 4 and 7. It is hard to map the removals > and additions and there are some changes that looks like changing behaviors, > but end up not being clear in the big patch. Also with big patch it is prune > to the rebasing and backport conflicts. I had the same concerns when I was writing this. I originally had it split up, but it seemed really cluttered with 2 sets of helpers (intel-internal + drm) that worked slightly differently. I'll try again now that some time has passed, perhaps a fresh look will help. Sean > > Would be possible to split some work in moving individual functions from i915 > to drm little by little with smaller patches? > > But thank you for this great work. It is also good to align our drm drivers. > > Thanks, > Rodrigo. > > > > > Thanks, > > > > Sean > > > > Link: https://patchwork.freedesktop.org/series/94623/ #v1 > > Link: https://patchwork.freedesktop.org/series/94713/ #v2 > > Link: https://patchwork.freedesktop.org/series/94712/ #v3 > > Link: https://patchwork.freedesktop.org/series/94712/ #v4 > > > > Sean Paul (10): > > drm/hdcp: Add drm_hdcp_atomic_check() > > drm/hdcp: Avoid changing crtc state in hdcp atomic check > > drm/hdcp: Update property value on content type and user changes > > drm/hdcp: Expand HDCP helper library for enable/disable/check > > drm/i915/hdcp: Consolidate HDCP setup/state cache > > drm/i915/hdcp: Retain hdcp_capable return codes > > drm/i915/hdcp: Use HDCP helpers for i915 > > dt-bindings: msm/dp: Add bindings for HDCP registers > > arm64: dts: qcom: sc7180: Add support for HDCP in dp-controller > > drm/msm: Implement HDCP 1.x using the new drm HDCP helpers > > > > .../bindings/display/msm/dp-controller.yaml |7 +- > > arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi |8 + > > arch/arm64/boot/dts/qcom/sc7180.dtsi |6 +- > > drivers/gpu/drm/drm_hdcp.c| 1197 - > > drivers/gpu/drm/i915/display/intel_atomic.c |7 +- > > drivers/gpu/drm/i915/display/intel_ddi.c | 29 +- > > .../drm/i915/display/intel_display_debugfs.c | 11 +- > > .../drm/i915/display/intel_display_types.h| 58 +- > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 345 ++--- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 17 +- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 1011 +++--- > > drivers/gpu/drm/i915/display/intel_hdcp.h | 36 +- > > drivers/gpu/drm/i915/display/intel_hdmi.c | 256 ++-- > > drivers/gpu/drm/msm/Makefile |1 + > > drivers/gpu/drm/msm/dp/dp_debug.c | 46 +- > > drivers/gpu/drm/msm/dp/dp_debug.h |6 +- > > drivers/gpu/drm/msm/dp/dp_display.c | 46 +- > > drivers/gpu/drm/msm/dp/dp_display.h |5 + > > drivers/gpu/drm/msm/dp/dp_drm.c | 68 +- > > drivers/gpu/drm/msm/dp/dp_drm.h |5 + > > drivers/gpu/drm/msm/dp/dp_hdcp.c | 453 +++ > > drivers/gpu/drm/msm/dp/dp_hdcp.h | 27 + > > drivers/gpu/drm/msm/dp/dp_parser.c| 20 +- > > drivers/gpu/drm/msm/dp/dp_parser.h|4 + > > drivers/gpu/drm/msm/dp/dp_reg.h | 32 +- > > drivers/gpu/drm/msm/msm_atomic.c | 15 + > > include/drm/drm_hdcp.h| 194 +++ > > 27 files changed, 2582 insertions(+), 1328 deletions(-) > > create mode 100644 drivers/gpu/drm/msm/dp/dp_hdcp.c > > create mode 100644 drivers/gpu/drm/msm/dp/dp_hdcp.h > > > > -- > > Sean Paul, Software Engineer, Google / Chromium OS > > -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH v2 4/5] drm/i915: ttm backend dont provide mmap_offset for kernel buffers
On 14/04/2022 15:05, Thomas Hellström wrote:
On Tue, 2022-04-12 at 15:18 +, Robert Beckett wrote:
stolen/kernel buffers should not be mmapable by userland.
do not provide callbacks to facilitate this for these buffers.
Signed-off-by: Robert Beckett
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 32 +--
--
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index a878910a563c..b20f81836c54 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -1092,8 +1092,8 @@ static void i915_ttm_unmap_virtual(struct
drm_i915_gem_object *obj)
ttm_bo_unmap_virtual(i915_gem_to_ttm(obj));
}
-static const struct drm_i915_gem_object_ops i915_gem_ttm_obj_ops = {
- .name = "i915_gem_object_ttm",
+static const struct drm_i915_gem_object_ops
i915_gem_ttm_user_obj_ops = {
+ .name = "i915_gem_object_ttm_user",
.flags = I915_GEM_OBJECT_IS_SHRINKABLE |
I915_GEM_OBJECT_SELF_MANAGED_SHRINK_LIST,
@@ -,6 +,21 @@ static const struct drm_i915_gem_object_ops
i915_gem_ttm_obj_ops = {
.mmap_ops = &vm_ops_ttm,
};
+static const struct drm_i915_gem_object_ops
i915_gem_ttm_kern_obj_ops = {
+ .name = "i915_gem_object_ttm_kern",
+ .flags = I915_GEM_OBJECT_IS_SHRINKABLE |
+ I915_GEM_OBJECT_SELF_MANAGED_SHRINK_LIST,
+
+ .get_pages = i915_ttm_get_pages,
+ .put_pages = i915_ttm_put_pages,
+ .truncate = i915_ttm_truncate,
+ .shrink = i915_ttm_shrink,
+
+ .adjust_lru = i915_ttm_adjust_lru,
+ .delayed_free = i915_ttm_delayed_free,
+ .migrate = i915_ttm_migrate,
+};
Do we really need two different ops here?
Since if we don't have mmap ops, basically that tells GEM it should do
the mmapping rather than TTM.
That might of course come in handy for the shmem backend, but I don't
fully follow why we need this for stolen.
the main rationale for doing this was to avoid
drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c:can_mmap() presuming
that is can use I915_MMAP_TYPE_FIXED
As the original backend also did not have mmap_offset handlers for
stolen, this seemed like a reasonable design.
If desired, we could add a special case for the testing logic, but those
special cases have tendency to multiply.
Also for the framebuffer handed over from BIOS to fbdev, Does that need
mmapping and if so, how do we handle that?
I'm not sure of the usecase there. Do you know of any igt test that
tests this? I can investigate further if you do not.
/Thomas
+
void i915_ttm_bo_destroy(struct ttm_buffer_object *bo)
{
struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
@@ -1165,10 +1180,19 @@ int __i915_gem_ttm_object_init(struct
intel_memory_region *mem,
.no_wait_gpu = false,
};
enum ttm_bo_type bo_type;
+ const struct drm_i915_gem_object_ops *ops;
int ret;
drm_gem_private_object_init(&i915->drm, &obj->base, size);
- i915_gem_object_init(obj, &i915_gem_ttm_obj_ops, &lock_class,
flags);
+
+ if (flags & I915_BO_ALLOC_USER &&
intel_region_to_ttm_type(mem) != I915_PL_STOLEN) {
+ bo_type = ttm_bo_type_device;
+ ops = &i915_gem_ttm_user_obj_ops;
+ } else {
+ bo_type = ttm_bo_type_kernel;
+ ops = &i915_gem_ttm_kern_obj_ops;
+ }
+ i915_gem_object_init(obj, ops, &lock_class, flags);
obj->bo_offset = offset;
@@ -1178,8 +1202,6 @@ int __i915_gem_ttm_object_init(struct
intel_memory_region *mem,
INIT_RADIX_TREE(&obj->ttm.get_io_page.radix, GFP_KERNEL |
__GFP_NOWARN);
mutex_init(&obj->ttm.get_io_page.lock);
- bo_type = (obj->flags & I915_BO_ALLOC_USER) ?
ttm_bo_type_device :
- ttm_bo_type_kernel;
obj->base.vma_node.driver_private = i915_gem_to_ttm(obj);
Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Use i915_probe_error instead of drm_err
On Thu, 14 Apr 2022, "Belgaumkar, Vinay" wrote:
> On 4/13/2022 11:41 PM, Anshuman Gupta wrote:
>> On 2022-04-13 at 04:18:52 +0530, Vinay Belgaumkar wrote:
>>> This will ensure we don't have false positives when we run
>>> error injection tests.
>>>
>>> Signed-off-by: Vinay Belgaumkar
>>> ---
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 42 ++---
>>> 1 file changed, 21 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>>> index b170238aa15c..639de3c10545 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
>>> @@ -152,8 +152,8 @@ static int slpc_query_task_state(struct intel_guc_slpc
>>> *slpc)
>>>
>>> ret = guc_action_slpc_query(guc, offset);
>>> if (unlikely(ret))
>> As commit logs describe, this code patch can hit, when we run error
>> injection test.
>> Do we need unlikely() here?
>> Br,
>> Anshuman Gupta.
>
> I think we still need the unlikely(). Majority of the time, we still
> need the compiler optimization.
>
> Only in the rare case of running the error injection test will it not be
> needed.
IMO most uses of likely/unlikely (and inline for that matter) are
premature optimization that just shouldn't be there. They're a
distraction to human readers, and a style that all too easily gets cargo
culted all over the place. In most cases, please just let the compiler
do what it does best.
And *then* if you still need to squeeze extra cycles out of the code,
and can quantify it, by all means optimize. But in that case, maybe the
root cause is somewhere other than a missing likely/unlikely/inline?
BR,
Jani.
>
> Thanks,
>
> Vinay.
>
>>> - drm_err(&i915->drm, "Failed to query task state (%pe)\n",
>>> - ERR_PTR(ret));
>>> + i915_probe_error(i915, "Failed to query task state (%pe)\n",
>>> +ERR_PTR(ret));
>>>
>>> drm_clflush_virt_range(slpc->vaddr, SLPC_PAGE_SIZE_BYTES);
>>>
>>> @@ -170,8 +170,8 @@ static int slpc_set_param(struct intel_guc_slpc *slpc,
>>> u8 id, u32 value)
>>>
>>> ret = guc_action_slpc_set_param(guc, id, value);
>>> if (ret)
>>> - drm_err(&i915->drm, "Failed to set param %d to %u (%pe)\n",
>>> - id, value, ERR_PTR(ret));
>>> + i915_probe_error(i915, "Failed to set param %d to %u (%pe)\n",
>>> +id, value, ERR_PTR(ret));
>>>
>>> return ret;
>>> }
>>> @@ -211,8 +211,8 @@ static int slpc_force_min_freq(struct intel_guc_slpc
>>> *slpc, u32 freq)
>>> SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
>>> freq);
>>> if (ret)
>>> - drm_err(&i915->drm, "Unable to force min freq to %u:
>>> %d",
>>> - freq, ret);
>>> + i915_probe_error(i915, "Unable to force min freq to %u:
>>> %d",
>>> +freq, ret);
>>> }
>>>
>>> return ret;
>>> @@ -247,9 +247,9 @@ int intel_guc_slpc_init(struct intel_guc_slpc *slpc)
>>>
>>> err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma, (void
>>> **)&slpc->vaddr);
>>> if (unlikely(err)) {
>>> - drm_err(&i915->drm,
>>> - "Failed to allocate SLPC struct (err=%pe)\n",
>>> - ERR_PTR(err));
>>> + i915_probe_error(i915,
>>> +"Failed to allocate SLPC struct (err=%pe)\n",
>>> +ERR_PTR(err));
>>> return err;
>>> }
>>>
>>> @@ -316,15 +316,15 @@ static int slpc_reset(struct intel_guc_slpc *slpc)
>>> ret = guc_action_slpc_reset(guc, offset);
>>>
>>> if (unlikely(ret < 0)) {
>>> - drm_err(&i915->drm, "SLPC reset action failed (%pe)\n",
>>> - ERR_PTR(ret));
>>> + i915_probe_error(i915, "SLPC reset action failed (%pe)\n",
>>> +ERR_PTR(ret));
>>> return ret;
>>> }
>>>
>>> if (!ret) {
>>> if (wait_for(slpc_is_running(slpc), SLPC_RESET_TIMEOUT_MS)) {
>>> - drm_err(&i915->drm, "SLPC not enabled! State = %s\n",
>>> - slpc_get_state_string(slpc));
>>> + i915_probe_error(i915, "SLPC not enabled! State = %s\n",
>>> +slpc_get_state_string(slpc));
>>> return -EIO;
>>> }
>>> }
>>> @@ -616,8 +616,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>>>
>>> ret = slpc_reset(slpc);
>>> if (unlikely(ret < 0)) {
>>> - drm_err(&i915->drm, "SLPC Reset event returned (%pe)\n",
>>> - ERR_PTR(ret));
>>> + i915_probe_error(i915, "SLPC Reset event returned (%pe)\n",
>>> +ERR_PTR(ret));
>>> return
RE: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Use i915_probe_error instead of drm_err
> -Original Message-
> From: Belgaumkar, Vinay
> Sent: Thursday, April 14, 2022 8:38 PM
> To: Gupta, Anshuman
> Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Use i915_probe_error
> instead of drm_err
>
>
> On 4/13/2022 11:41 PM, Anshuman Gupta wrote:
> > On 2022-04-13 at 04:18:52 +0530, Vinay Belgaumkar wrote:
> >> This will ensure we don't have false positives when we run error
> >> injection tests.
> >>
> >> Signed-off-by: Vinay Belgaumkar
> >> ---
> >> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 42 ++---
> >> 1 file changed, 21 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> >> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> >> index b170238aa15c..639de3c10545 100644
> >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> >> @@ -152,8 +152,8 @@ static int slpc_query_task_state(struct
> >> intel_guc_slpc *slpc)
> >>
> >>ret = guc_action_slpc_query(guc, offset);
> >>if (unlikely(ret))
> > As commit logs describe, this code patch can hit, when we run error
> > injection
> test.
> > Do we need unlikely() here?
> > Br,
> > Anshuman Gupta.
>
> I think we still need the unlikely(). Majority of the time, we still need the
> compiler optimization.
>
> Only in the rare case of running the error injection test will it not be
> needed.
Reviewed-by: Anshuman Gupta
>
> Thanks,
>
> Vinay.
>
> >> - drm_err(&i915->drm, "Failed to query task state (%pe)\n",
> >> - ERR_PTR(ret));
> >> + i915_probe_error(i915, "Failed to query task state (%pe)\n",
> >> + ERR_PTR(ret));
> >>
> >>drm_clflush_virt_range(slpc->vaddr, SLPC_PAGE_SIZE_BYTES);
> >>
> >> @@ -170,8 +170,8 @@ static int slpc_set_param(struct intel_guc_slpc
> >> *slpc, u8 id, u32 value)
> >>
> >>ret = guc_action_slpc_set_param(guc, id, value);
> >>if (ret)
> >> - drm_err(&i915->drm, "Failed to set param %d to %u (%pe)\n",
> >> - id, value, ERR_PTR(ret));
> >> + i915_probe_error(i915, "Failed to set param %d to %u (%pe)\n",
> >> + id, value, ERR_PTR(ret));
> >>
> >>return ret;
> >> }
> >> @@ -211,8 +211,8 @@ static int slpc_force_min_freq(struct intel_guc_slpc
> *slpc, u32 freq)
> >>
> SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> >> freq);
> >>if (ret)
> >> - drm_err(&i915->drm, "Unable to force min freq to %u:
> %d",
> >> - freq, ret);
> >> + i915_probe_error(i915, "Unable to force min freq to
> %u: %d",
> >> + freq, ret);
> >>}
> >>
> >>return ret;
> >> @@ -247,9 +247,9 @@ int intel_guc_slpc_init(struct intel_guc_slpc
> >> *slpc)
> >>
> >>err = intel_guc_allocate_and_map_vma(guc, size, &slpc->vma, (void
> **)&slpc->vaddr);
> >>if (unlikely(err)) {
> >> - drm_err(&i915->drm,
> >> - "Failed to allocate SLPC struct (err=%pe)\n",
> >> - ERR_PTR(err));
> >> + i915_probe_error(i915,
> >> + "Failed to allocate SLPC struct (err=%pe)\n",
> >> + ERR_PTR(err));
> >>return err;
> >>}
> >>
> >> @@ -316,15 +316,15 @@ static int slpc_reset(struct intel_guc_slpc *slpc)
> >>ret = guc_action_slpc_reset(guc, offset);
> >>
> >>if (unlikely(ret < 0)) {
> >> - drm_err(&i915->drm, "SLPC reset action failed (%pe)\n",
> >> - ERR_PTR(ret));
> >> + i915_probe_error(i915, "SLPC reset action failed (%pe)\n",
> >> + ERR_PTR(ret));
> >>return ret;
> >>}
> >>
> >>if (!ret) {
> >>if (wait_for(slpc_is_running(slpc), SLPC_RESET_TIMEOUT_MS)) {
> >> - drm_err(&i915->drm, "SLPC not enabled! State = %s\n",
> >> - slpc_get_state_string(slpc));
> >> + i915_probe_error(i915, "SLPC not enabled! State =
> %s\n",
> >> + slpc_get_state_string(slpc));
> >>return -EIO;
> >>}
> >>}
> >> @@ -616,8 +616,8 @@ int intel_guc_slpc_enable(struct intel_guc_slpc
> >> *slpc)
> >>
> >>ret = slpc_reset(slpc);
> >>if (unlikely(ret < 0)) {
> >> - drm_err(&i915->drm, "SLPC Reset event returned (%pe)\n",
> >> - ERR_PTR(ret));
> >> + i915_probe_error(i915, "SLPC Reset event returned (%pe)\n",
> >> + ERR_PTR(ret));
> >>return ret;
> >>}
> >>
> >> @@ -632,24 +632,24 @@ int intel_guc_slpc_enable(struct intel_guc_slpc
> *slpc)
> >>/* Ignore efficient freq and set min to platform min */
> >>ret = slpc_ignore_eff_freq(slpc, true);
> >>if (unlikely(ret)) {
> >> - drm_err
Re: [PATCH v2] drm/msm/dp: enhance both connect and disconnect pending_timeout handle
On 4/13/2022 5:02 PM, Stephen Boyd wrote:
The subject is still misleading. It is fixing something. It may be
enhancing it as well but it is clearly fixing it first.
Quoting Kuogee Hsieh (2022-04-06 14:28:13)
dp_hpd_plug_handle() is responsible for setting up main link and send
uevent to notify user space framework to start video stream. Similarly,
dp_hdp_unplug_handle is responsible to send uevent to notify user space
framework to stop video stream and then tear down main link.
However there are rare cases, such as in the middle of system suspending,
that uevent could not be delivered to user space framework. Therefore
some kind of recover mechanism armed by timer need to be in place in the
case of user space framework does not respond to uevent.
This patch have both dp_conenct_pending_timeout and
dp_disconnect_pending_timeout are used to stop video stream and tear down
main link and eventually restore DP driver state to known default
DISCONNECTED state in the case of timer fired due to framework does not
respond to uevent so that DP driver can recover itself gracefully at next
dongle unplug followed by plugin event.
Changes in v2:
-- replace dp_display_usbpd_disconnect_cb with dp_display_notify_disconnect
I'd prefer this part to be a different patch. It can come after the fix
to ease backporting.
Also, is there any response to Dmitry's question yet? I haven't seen
anything.
Sorry, since our internal review does not like this approach.
I will upload new patch for review soon.
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 2433edb..ffafe17 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -22,6 +22,7 @@ struct dp_ctrl {
int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl);
int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl);
int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
+int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl);
int dp_ctrl_off(struct dp_ctrl *dp_ctrl);
void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl);
void dp_ctrl_isr(struct dp_ctrl *dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 178b774..a6200a5 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -451,11 +451,14 @@ static int dp_display_usbpd_configure_cb(struct device
*dev)
static int dp_display_usbpd_disconnect_cb(struct device *dev)
We shouldn't need to keep around an empty function.
{
+ return 0;
+}
+
+static void dp_display_notify_disconnect(struct device *dev)
+{
struct dp_display_private *dp = dev_get_dp_display_private(dev);
dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);
-
- return 0;
}
static void dp_display_handle_video_request(struct dp_display_private *dp)
@@ -593,10 +596,16 @@ static int dp_connect_pending_timeout(struct
dp_display_private *dp, u32 data)
mutex_lock(&dp->event_mutex);
+ /*
+* main link had been setup but video is not ready yet
+* only tear down main link
+*/
state = dp->hpd_state;
if (state == ST_CONNECT_PENDING) {
- dp->hpd_state = ST_CONNECTED;
DRM_DEBUG_DP("type=%d\n", dp->dp_display.connector_type);
+ dp_ctrl_off_link(dp->ctrl);
+ dp_display_host_phy_exit(dp);
+ dp->hpd_state = ST_DISCONNECTED;
}
mutex_unlock(&dp->event_mutex);
@@ -645,6 +654,7 @@ static int dp_hpd_unplug_handle(struct dp_display_private
*dp, u32 data)
if (dp->link->sink_count == 0) {
dp_display_host_phy_exit(dp);
}
+ dp_display_notify_disconnect(&dp->pdev->dev);
mutex_unlock(&dp->event_mutex);
return 0;
}
@@ -661,19 +671,22 @@ static int dp_hpd_unplug_handle(struct dp_display_private
*dp, u32 data)
return 0;
}
- dp->hpd_state = ST_DISCONNECT_PENDING;
-
/* disable HPD plug interrupts */
dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_PLUG_INT_MASK,
false);
/*
* We don't need separate work for disconnect as
* connect/attention interrupts are disabled
-*/
- dp_display_usbpd_disconnect_cb(&dp->pdev->dev);
+ */
This comment end is wrong. It should be unchanged.
+ dp_display_notify_disconnect(&dp->pdev->dev);
- /* start sentinel checking in case of missing uevent */
- dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0, DP_TIMEOUT_5_SECOND);
+ if (state == ST_DISPLAY_OFF) {
+ dp->hpd_state = ST_DISCONNECTED;
+ } else {
+ /* start sentinel checking in case of missing uevent */
+ dp_add_event(dp, EV_DISCONNECT_PENDING_TIMEOUT, 0,
DP_TIMEOUT_5_SECOND);
+ dp->hpd_state = ST_DISCONNECT_PENDING;
+ }
/* signal the disconnect event early to ensure prope
Re: [PATCH v7 1/4] drm/msm/dp: Add eDP support via aux_bus
Hi,
On Thu, Apr 14, 2022 at 5:20 AM Sankeerth Billakanti
wrote:
>
> @@ -1530,6 +1532,60 @@ void msm_dp_debugfs_init(struct msm_dp *dp_display,
> struct drm_minor *minor)
> }
> }
>
> +static int dp_display_get_next_bridge(struct msm_dp *dp)
> +{
> + int rc;
> + struct dp_display_private *dp_priv;
> + struct device_node *aux_bus;
> + struct device *dev;
> +
> + dp_priv = container_of(dp, struct dp_display_private, dp_display);
> + dev = &dp_priv->pdev->dev;
> + aux_bus = of_get_child_by_name(dev->of_node, "aux-bus");
> +
> + if (aux_bus && dp->is_edp) {
> + dp_display_host_init(dp_priv);
> + dp_catalog_ctrl_hpd_config(dp_priv->catalog);
> + dp_display_host_phy_init(dp_priv);
> + enable_irq(dp_priv->irq);
> +
> + rc = devm_of_dp_aux_populate_ep_devices(dp_priv->aux);
> + of_node_put(aux_bus);
> + if (rc) {
> + disable_irq(dp_priv->irq);
> + dp_display_host_phy_exit(dp_priv);
> + dp_display_host_deinit(dp_priv);
> + return rc;
> + }
> + } else if (dp->is_edp) {
> + DRM_ERROR("eDP aux_bus not found\n");
> + return -ENODEV;
> + }
> +
> + /*
> +* External bridges are mandatory for eDP interfaces: one has to
> +* provide at least an eDP panel (which gets wrapped into
> panel-bridge).
> +*
> +* For DisplayPort interfaces external bridges are optional, so
> +* silently ignore an error if one is not present (-ENODEV).
> +*/
> + rc = dp_parser_find_next_bridge(dp_priv->parser);
This gets into the same problem that Dmitry pointed out that ps8640
has that's addressed by my recent series [1]. Namely it's not
guaranteed that the panel will have finished probing by the time
devm_of_dp_aux_populate_ep_devices() finishes probing. I don't think
it's going to be really solvable without the bigger rewrite that we've
been discussing, though. ...it's probably OK to land something like
what you have here, but it might at least deserve a comment in the
code?
[1] https://lore.kernel.org/r/20220409023628.2104952-1-diand...@chromium.org
> + if (rc == -ENODEV) {
> + if (dp->is_edp) {
> + DRM_ERROR("eDP: next bridge is not present\n");
> + return rc;
> + }
> + } else if (rc) {
> + if (rc != -EPROBE_DEFER)
> + DRM_ERROR("DP: error parsing next bridge: %d\n", rc);
> + return rc;
In both of your two error returns here isn't it a problem that you don't do:
disable_irq(dp_priv->irq);
dp_display_host_phy_exit(dp_priv);
dp_display_host_deinit(dp_priv);
Should probably at least fix that clear error before landing, unless
I'm misunderstanding and there's some reason not to do that?
As discussed previously, I'm not convinced that we've covered every
corner case for properly doing and undoing the above things. I'm
hoping that once we do the cleanup and move to pm_runtime() management
that it will be cleaned up?
> @@ -114,10 +114,12 @@ struct drm_bridge *dp_bridge_init(struct msm_dp
> *dp_display, struct drm_device *
> bridge->funcs = &dp_bridge_ops;
> bridge->type = dp_display->connector_type;
>
> - bridge->ops =
> - DRM_BRIDGE_OP_DETECT |
> - DRM_BRIDGE_OP_HPD |
> - DRM_BRIDGE_OP_MODES;
> + if (!dp_display->is_edp) {
> + bridge->ops =
> + DRM_BRIDGE_OP_DETECT |
> + DRM_BRIDGE_OP_HPD |
> + DRM_BRIDGE_OP_MODES;
Given that Dmitry had questions about why eDP has different ops in his
previous review of this code, the above probably deserves an inline
code comment. If you want to use my wording, you could paste this into
your code:
/*
* Many ops only make sense for DP. Why?
* - Detect/HPD are used by DRM to know if a display is _physically_
* there, not whether the display is powered on / finished initting.
* On eDP we assume the display is always there because you can't
* know until power is applied. If we don't implement the ops DRM will
* assume our display is always there.
* - Currently eDP mode reading is driven by the panel driver. This
* allows the panel driver to properly power itself on to read the
* modes.
*/
Overall: as discussed, I think that the current implementation is a
bit fragile and might have some wrong corner cases since it's hard for
me to reason about exactly when we init/de-init things. Even if it
works great, the fact that it's hard to reason about isn't wonderful.
That being said, I honestly believe that would benefit upstream to get
this landed and iterate on it. I don't think this should be causing
any existing behavior to be
Re: [PATCH v7 2/4] drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP
Hi,
On Thu, Apr 14, 2022 at 5:20 AM Sankeerth Billakanti
wrote:
>
> The panel-edp enables the eDP panel power during probe, get_modes
> and enable.
Technically the panel-edp powers on the panel in pre_enable()
> The eDP connect and disconnect interrupts for the eDP/DP
> controller are directly dependent on panel power. As eDP display can be
> assumed as always connected, the controller driver can skip the eDP
> connect and disconnect interrupts. Any disruption in the link status
> will be indicated via the IRQ_HPD interrupts.
>
> So, the eDP controller driver can just enable the IRQ_HPD and replug
> interrupts. The DP controller driver still needs to enable all the
> interrupts.
>
> Signed-off-by: Sankeerth Billakanti
> ---
>
> Changes in v7:
> - reordered the patch in the series
> - modified the return statement for isr
> - connector check modified to just check for eDP
>
> drivers/gpu/drm/msm/dp/dp_catalog.c | 9 +++--
> drivers/gpu/drm/msm/dp/dp_display.c | 22 +-
> 2 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index fac815f..07f2389 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -569,10 +569,6 @@ void dp_catalog_ctrl_hpd_config(struct dp_catalog
> *dp_catalog)
>
> u32 reftimer = dp_read_aux(catalog, REG_DP_DP_HPD_REFTIMER);
>
> - /* enable HPD plug and unplug interrupts */
> - dp_catalog_hpd_config_intr(dp_catalog,
> - DP_DP_HPD_PLUG_INT_MASK | DP_DP_HPD_UNPLUG_INT_MASK, true);
> -
> /* Configure REFTIMER and enable it */
> reftimer |= DP_DP_HPD_REFTIMER_ENABLE;
> dp_write_aux(catalog, REG_DP_DP_HPD_REFTIMER, reftimer);
> @@ -599,13 +595,14 @@ u32 dp_catalog_hpd_get_intr_status(struct dp_catalog
> *dp_catalog)
> {
> struct dp_catalog_private *catalog = container_of(dp_catalog,
> struct dp_catalog_private, dp_catalog);
> - int isr = 0;
> + int isr, mask;
>
> isr = dp_read_aux(catalog, REG_DP_DP_HPD_INT_STATUS);
> dp_write_aux(catalog, REG_DP_DP_HPD_INT_ACK,
> (isr & DP_DP_HPD_INT_MASK));
> + mask = dp_read_aux(catalog, REG_DP_DP_HPD_INT_MASK);
>
> - return isr;
> + return isr & (mask | ~DP_DP_HPD_INT_MASK);
Please add a comment above this explaining what the goal of the above
statement is. I guess it's something like this, though you might want
to modify it to remove snark and insert the real reason unless you
like being snarky:
/*
* Report the raw status of all interrupts (AKA we still report the
* interrupt as asserted even if it's masked) _except_ for HPD-related.
* interrupts. We only report HPD-related interrupts if they're
* unmasked. We do it this way because we thought it would be extra
* confusing for readers of this code and we were bribed by Mordac to
* confuse you. OK, maybe that's not true. We actually do it this way
* because of .
*/
Along the same lines as my comments in patch #1, I don't have a great
feel for exactly when the various HPD bits are enabled / disabled and
it feels like it need to be made super obvious / well documented. That
being said, I'd be OK w/ that happening in the proposed cleanup.
-Doug
Re: [PATCH v7 3/4] drm/msm/dp: wait for hpd high before aux transaction
Hi,
On Thu, Apr 14, 2022 at 5:20 AM Sankeerth Billakanti
wrote:
>
> The source device should ensure the sink is ready before proceeding to
> read the sink capability or perform any aux transactions. The sink
> will indicate its readiness by asserting the HPD line. The controller
> driver needs to wait for the hpd line to be asserted by the sink before
> it performs any aux transactions.
>
> The eDP sink is assumed to be always connected. It needs power from the
> source and its HPD line will be asserted only after the panel is powered
> on. The panel power will be enabled from the panel-edp driver and only
> after that, the hpd line will be asserted.
>
> Whereas for DP, the sink can be hotplugged and unplugged anytime. The hpd
> line gets asserted to indicate the sink is connected and ready. Hence
> there is no need to wait for the hpd line to be asserted for a DP sink.
>
> Signed-off-by: Sankeerth Billakanti
> ---
It might be worth mentioning "after the cut" that we may eventually
end up changing the rules if people like my proposal [1]. However,
what your code is doing here for eDP is correct as things are
currently intended to work and it would make sense to land it while we
debate about whether we want to add the is_hpd_asserted() callback
like my patch does.
[1]
https://lore.kernel.org/r/20220408193536.RFC.3.Icf57bb12233a47727013c6ab69eebf803e22ebc1@changeid/
> Changes in v7:
> - add a comment to say why the wait si done for eDP
> - correct the commit text
>
> Changes in v6:
> - Wait for hpd high only for eDP
> - Split into smaller patches
>
> drivers/gpu/drm/msm/dp/dp_aux.c | 21 -
> drivers/gpu/drm/msm/dp/dp_aux.h | 3 ++-
> drivers/gpu/drm/msm/dp/dp_catalog.c | 13 +
> drivers/gpu/drm/msm/dp/dp_catalog.h | 1 +
> drivers/gpu/drm/msm/dp/dp_display.c | 2 +-
> 5 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
> index 6d36f63..cf0739f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_aux.c
> +++ b/drivers/gpu/drm/msm/dp/dp_aux.c
> @@ -36,6 +36,7 @@ struct dp_aux_private {
> bool initted;
> u32 offset;
> u32 segment;
> + bool is_edp;
Kinda nitty, but can you put it next to the other booleans? This will
help with structure packing.
> struct drm_dp_aux dp_aux;
> };
> @@ -337,6 +338,22 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
> goto exit;
> }
>
> + /*
> +* For eDP it's important to give a reasonably long wait here for HPD
> +* to be asserted. This is because the panel driver may have _just_
> +* turned on the panel and then tried to do an AUX transfer. The panel
> +* driver has no way of knowing when the panel is ready, so it's up
> +* to us to wait. For DP we never get into this situation so let's
> +* avoid ever doing the extra long wait for DP.
> +*/
> + if (aux->is_edp) {
> + ret = dp_catalog_aux_wait_for_hpd_connect_state(aux->catalog);
> + if (ret) {
> + DRM_DEBUG_DP("Panel not ready for aux
> transactions\n");
> + goto exit;
> + }
> + }
> +
> dp_aux_update_offset_and_segment(aux, msg);
> dp_aux_transfer_helper(aux, msg, true);
>
> @@ -491,7 +508,8 @@ void dp_aux_unregister(struct drm_dp_aux *dp_aux)
> drm_dp_aux_unregister(dp_aux);
> }
>
> -struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog)
> +struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog,
> + bool is_edp)
nit: I think indentation rules for this file are that the type of the
argument for the 2nd line should line up right under the 1st. Thus you
should delete one tab character and insert 6 spaces before the "bool".
Similar in other places, like your header file.
Stuff above is all nits and this looks right to me. I'm happy with:
Reviewed-by: Douglas Anderson
Re: [PATCH v7 4/4] Support the eDP modes given by panel
Hi, On Thu, Apr 14, 2022 at 5:20 AM Sankeerth Billakanti wrote: > > The eDP controller does not have a reliable way keep panel > powered on to read the sink capabilities. So, the controller > driver cannot validate if a mode can be supported by the > source. We will rely on the panel driver to populate only > the supported modes for now. > > Signed-off-by: Sankeerth Billakanti > --- > drivers/gpu/drm/msm/dp/dp_display.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index c7277f0..0f18a16 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -998,6 +998,14 @@ enum drm_mode_status dp_bridge_mode_valid(struct > drm_bridge *bridge, > return -EINVAL; > } > > + /* > +* The eDP controller currently does not have a reliable way of > +* enabling panel power to read sink capabilities. So, we rely > +* on the panel driver to populate only supported modes for now. > +*/ > + if (dp->is_edp) > + return MODE_OK; As discussed out-of-band, I agree that this is the right thing for now and making this assumption won't break anything. In general the set of eDP panels is known ahead of time it's fairly unlikely someone would set things up so that a panel couldn't use the mode it was reporting. Longer term we should figure out a way to solve this but it doesn't have to be today. To properly implement mode_valid() we've got to combine knowledge from the panel (mostly rates supported and number of lanes supported) with the controller (rates supported, number of lanes supported/hooked up on this board). In any case: Reviewed-by: Douglas Anderson
Re: [PATCH v7 0/4] Add support for the eDP panel over aux_bus
Hi, On Thu, Apr 14, 2022 at 5:19 AM Sankeerth Billakanti wrote: > > This series adds support for generic eDP panel over aux_bus. > > These changes are dependent on the following series: > https://patchwork.kernel.org/project/linux-arm-msm/list/?series=613654&state=* You're basically depending on the last two patches of that series. What's the plan there? In patchwork they're marked as "Not Applicable". If they're good to go, maybe we should land them? If not, maybe you should include them (with Dmitry as the author, of course) at the beginning of your series? > Sankeerth Billakanti (4): > drm/msm/dp: Add eDP support via aux_bus > drm/msm/dp: Support only IRQ_HPD and REPLUG interrupts for eDP > drm/msm/dp: wait for hpd high before aux transaction > Support the eDP modes given by panel One of these things is not like the others. One of these things just doesn't belong. Can you spot which patch is missing the prefix by looking at the subject line of all 4 patches? ;-)
Re: [PATCH v2] drm/msm/dp: stop event kernel thread when DP unbind
On 4/13/2022 4:19 PM, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2022-04-13 14:04:25)
Current DP driver implementation, event thread is kept running
after DP display is unbind. This patch fix this problem by disabling
DP irq and stop event thread to exit gracefully at dp_display_unbind().
Changes in v2:
-- start event thread at dp_display_bind()
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dp/dp_display.c | 40 +++--
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..943e4f1 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -113,6 +113,7 @@ struct dp_display_private {
u32 hpd_state;
u32 event_pndx;
u32 event_gndx;
+ struct task_struct *ev_tsk;
struct dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
@@ -230,6 +231,31 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(&dp->audio_comp);
}
+static int hpd_event_thread(void *data);
Is there a reason why this is needed vs. defining the kthread start
function after hpd_event_thread()?
too many code need to be relocated.
+
+static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
Maybe dp_hpd_event_thread_start()?
+{
+ init_waitqueue_head(&dp_priv->event_q);
+ spin_lock_init(&dp_priv->event_lock);
+
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+
+ if (IS_ERR(dp_priv->ev_tsk))
+ DRM_ERROR("failed to create DP event thread\n");
Can we return an error from this function?
+}
+
+static void dp_hpd_event_stop(struct dp_display_private *dp_priv)
Maybe dp_hpd_event_thread_stop()?
+{
+ if (IS_ERR(dp_priv->ev_tsk))
+ return;
If we handled the error then this check becomes impossible.
+
+ kthread_stop(dp_priv->ev_tsk);
+
+ /* reset event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
+}
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -269,6 +295,7 @@ static int dp_display_bind(struct device *dev, struct
device *master,
if (rc)
DRM_ERROR("Audio registration Dp failed\n");
+ dp_hpd_event_setup(dp); /* start event thread */
The comment is useless, please remove.
end:
return rc;
}
@@ -280,6 +307,8 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
+ disable_irq(dp->irq);
Is the disable_irq() necessary? It would be nicer to silence the
hardware and remove the disable_irq() so that we can reason about the
code assuming the irq is always enabled after it is requested.
+ dp_hpd_event_stop(dp); /* stop event thread */
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
Re: [PATCH v3 5/5] drm/i915: stolen memory use ttm backend
On 14/04/2022 15:41, Thomas Hellström wrote:
Hi,
On 4/12/22 21:38, Robert Beckett wrote:
+struct ttm_resource *
+i915_gem_stolen_reserve_range(struct drm_i915_private *i915,
+ resource_size_t size,
+ u64 start, u64 end)
{
- int ret;
+ struct intel_memory_region *mem = i915->mm.stolen_region;
- if (!drm_mm_initialized(&i915->mm.stolen))
- return -ENODEV;
-
- /* WaSkipStolenMemoryFirstPage:bdw+ */
- if (GRAPHICS_VER(i915) >= 8 && start < 4096)
- start = 4096;
Did we lose this Workaround?
woops, looks like we did. Nice catch.
I'll add a reservation at start of day if wa is needed when I issue v4.
Need to continue reviewing this patchnext week.
Thx,
Thomas
-
- mutex_lock(&i915->mm.stolen_lock);
- ret = drm_mm_insert_node_in_range(&i915->mm.stolen, node,
- size, alignment, 0,
- start, end, DRM_MM_INSERT_BEST);
- mutex_unlock(&i915->mm.stolen_lock);
+ if (!mem)
+ return ERR_PTR(-ENODEV);
+ return intel_region_ttm_resource_alloc(mem, size, start, end,
I915_BO_ALLOC_CONTIGUOUS);
+}
-
[PATCH v3] drm/msm/dp: stop event kernel thread when DP unbind
Current DP driver implementation, event thread is kept running
after DP display is unbind. This patch fix this problem by disabling
DP irq and stop event thread to exit gracefully at dp_display_unbind().
Changes in v2:
-- start event thread at dp_display_bind()
Changes in v3:
-- disable all HDP interrupts at unbind
-- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
-- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
-- move init_waitqueue_head(&dp->event_q) to probe()
-- move spin_lock_init(&dp->event_lock) to probe()
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dp/dp_display.c | 42 -
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..0b9a96f 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -113,6 +113,7 @@ struct dp_display_private {
u32 hpd_state;
u32 event_pndx;
u32 event_gndx;
+ struct task_struct *ev_tsk;
struct dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
@@ -230,6 +231,29 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(&dp->audio_comp);
}
+static int hpd_event_thread(void *data);
+
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
+{
+ int err = 0;
+
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+ if (IS_ERR(dp_priv->ev_tsk)) {
+ DRM_ERROR("failed to create DP event thread\n");
+ err = PTR_ERR(dp_priv->ev_tsk);
+ }
+ return err;
+}
+
+static void dp_hpd_event_thread_stop(struct dp_display_private *dp_priv)
+{
+ kthread_stop(dp_priv->ev_tsk);
+
+ /* reset event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
+}
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -269,6 +293,7 @@ static int dp_display_bind(struct device *dev, struct
device *master,
if (rc)
DRM_ERROR("Audio registration Dp failed\n");
+ rc = dp_hpd_event_thread_start(dp);
end:
return rc;
}
@@ -280,6 +305,9 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
+ /* disable all HPD interrupts */
+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
+ dp_hpd_event_thread_stop(dp);
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1082,7 @@ static int hpd_event_thread(void *data)
dp_priv = (struct dp_display_private *)data;
- while (1) {
+ while (!kthread_should_stop()) {
if (timeout_mode) {
wait_event_timeout(dp_priv->event_q,
(dp_priv->event_pndx == dp_priv->event_gndx),
@@ -1132,13 +1160,6 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
-{
- init_waitqueue_head(&dp_priv->event_q);
- spin_lock_init(&dp_priv->event_lock);
-
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
-}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
{
@@ -1266,7 +1287,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(&dp->event_mutex);
+ init_waitqueue_head(&dp->event_q);
+ spin_lock_init(&dp->event_lock);
/* Store DP audio handle inside DP display */
dp->dp_display.dp_audio = dp->audio;
@@ -1441,8 +1465,6 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
dp = container_of(dp_display, struct dp_display_private, dp_display);
- dp_hpd_event_setup(dp);
-
dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v2 1/1] i915/gem: drop wbinvd_on_all_cpus usage
Previous concern with using drm_clflush_sg was that we don't know what the sg_table is pointing to, thus the usage of wbinvd_on_all_cpus to flush everything at once to avoid paranoia. To make i915 more architecture-neutral and be less paranoid, lets attempt to use drm_clflush_sg to flush the pages for when the GPU wants to read from main memory. Signed-off-by: Michael Cheng --- drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c index f5062d0c6333..b0a5baaebc43 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c @@ -8,6 +8,7 @@ #include #include #include +#include #include @@ -250,16 +251,10 @@ static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj) * DG1 is special here since it still snoops transactions even with * CACHE_NONE. This is not the case with other HAS_SNOOP platforms. We * might need to revisit this as we add new discrete platforms. -* -* XXX: Consider doing a vmap flush or something, where possible. -* Currently we just do a heavy handed wbinvd_on_all_cpus() here since -* the underlying sg_table might not even point to struct pages, so we -* can't just call drm_clflush_sg or similar, like we do elsewhere in -* the driver. */ if (i915_gem_object_can_bypass_llc(obj) || (!HAS_LLC(i915) && !IS_DG1(i915))) - wbinvd_on_all_cpus(); + drm_clflush_sg(pages); sg_page_sizes = i915_sg_dma_sizes(pages->sgl); __i915_gem_object_set_pages(obj, pages, sg_page_sizes); -- 2.25.1
