Re: [PATCH] drm/bridge: anx7625: Fill in empty ELD when no connector
On Thu, Apr 14, 2022 at 05:00:04PM +0800, Hsin-Yi Wang wrote:
> 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;
Hi Hsin-Yi, it's OK for me.
Reviewed-by: Xin Ji
Thanks,
Xin
> }
>
> --
> 2.35.1.1178.g4f1659d476-goog
[PATCH resend 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 = >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_handle,
[PATCH resend 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 resend 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 = < IMX8MQ_CLK_DSI_PHY_REF>;
- clock-names = "phy_ref";
- reg = <0x30a00300 0x100>;
- power-domains = <_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 = < IMX8MQ_CLK_DSI_PHY_REF>;
+clock-names = "phy_ref";
+assigned-clocks = < IMX8MQ_CLK_DSI_PHY_REF>;
+assigned-clock-parents = < IMX8MQ_VIDEO_PLL1_OUT>;
+assigned-clock-rates = <2400>;
+#phy-cells = <0>;
+power-domains = <_mipi>;
+};
--
2.25.1
[PATCH resend 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 resend 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:
* Resend with Andrzej's and Jernej's mail addressed updated.
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
[PATCH resend 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 '*'. * Resend with reviewer mail addresses updated for patch 1/5. 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
Re: [PATCH] drm/bridge: fix anx6345 power up sequence
On Sun, Apr 17, 2022 at 11:52 AM Vasily Khoruzhick wrote: > > On Sun, Apr 17, 2022 at 9:15 AM Torsten Duwe wrote: > > > > Align the power-up sequence with the known-good procedure documented in [1]: > > un-swap dvdd12 and dvdd25, and allow a little extra time for them to settle > > before de-asserting reset. > > Hi Torsten, > > Interesting find! I tried to fix the issue several times by playing > with the delays to no avail. > > What's interesting, ANX6345 datasheet allows DVDD12 to come up either > earlier or later than DVDD25 with the delay of T1 (2ms typical) > between them, and actually bringing up DVDD12 first works fine in > u-boot. > > The datasheet also requires reset to be deasserted no earlier than T2 > (2-5ms) after all the rails are stable. > > Another thing it mentions is that the system clock must be stable for > T3 (1-3ms) before reset is deasserted, T3 is already a part of T2, > however it cannot be gated on Pinebook, see [1], page 15 > > The change looks good to me, but I'll need some time to actually test > it. If you don't hear from me for longer than a week please ping me. Your change doesn't fix the issue for me. Running "xrandr --output eDP-1 --off; xrandr --output eDP-1 --auto" in a loop triggers the issue pretty quickly even with the patch.
Re: [PATCH v2] drm/msm: properly add and remove internal bridges
On 4/11/2022 4:49 PM, Dmitry Baryshkov wrote:
Add calls to drm_bridge_add()/drm_bridge_remove() DRM bridges created by
the driver. This fixes the following warning.
WARNING: CPU: 0 PID: 1 at kernel/locking/mutex.c:579 __mutex_lock+0x840/0x9f4
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.18.0-rc1-2-g3054695a0d27-dirty
#55
Hardware name: Generic DT based system
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x58/0x70
dump_stack_lvl from __warn+0xc8/0x1e8
__warn from warn_slowpath_fmt+0x78/0xa8
warn_slowpath_fmt from __mutex_lock+0x840/0x9f4
__mutex_lock from mutex_lock_nested+0x1c/0x24
mutex_lock_nested from drm_bridge_hpd_enable+0x2c/0x84
drm_bridge_hpd_enable from msm_hdmi_modeset_init+0xc0/0x21c
msm_hdmi_modeset_init from mdp4_kms_init+0x53c/0x90c
mdp4_kms_init from msm_drm_bind+0x514/0x698
msm_drm_bind from try_to_bring_up_aggregate_device+0x160/0x1bc
try_to_bring_up_aggregate_device from
component_master_add_with_match+0xc4/0xf8
component_master_add_with_match from msm_pdev_probe+0x274/0x350
msm_pdev_probe from platform_probe+0x5c/0xbc
platform_probe from really_probe.part.0+0x9c/0x290
really_probe.part.0 from __driver_probe_device+0xa8/0x13c
__driver_probe_device from driver_probe_device+0x34/0x10c
driver_probe_device from __driver_attach+0xbc/0x178
__driver_attach from bus_for_each_dev+0x74/0xc0
bus_for_each_dev from bus_add_driver+0x160/0x1e4
bus_add_driver from driver_register+0x88/0x118
driver_register from do_one_initcall+0x6c/0x334
do_one_initcall from kernel_init_freeable+0x1bc/0x220
kernel_init_freeable from kernel_init+0x18/0x12c
kernel_init from ret_from_fork+0x14/0x2c
Fixes: 3d3f8b1f8b62 ("drm/bridge: make bridge registration independent of drm
flow")
Reported-by: kernel test robot
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Abhinav Kumar
---
Changes since v1:
- Dropped drm_bridge_detach() call, it is not necessary, also it breaks
compilation if MSM DRM is built as module.
---
drivers/gpu/drm/msm/dp/dp_drm.c| 4
drivers/gpu/drm/msm/dsi/dsi_manager.c | 3 +++
drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 3 +++
drivers/gpu/drm/msm/msm_drv.c | 3 +++
4 files changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index 80f59cf99089..262744914f97 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -230,9 +230,13 @@ struct drm_bridge *msm_dp_bridge_init(struct msm_dp
*dp_display, struct drm_devi
bridge->funcs = _bridge_ops;
bridge->encoder = encoder;
+ drm_bridge_add(bridge);
+
rc = drm_bridge_attach(encoder, bridge, NULL,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (rc) {
DRM_ERROR("failed to attach bridge, rc=%d\n", rc);
+ drm_bridge_remove(bridge);
+
return ERR_PTR(rc);
}
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 9f6af0f0fe00..1db93e562fe6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -665,6 +665,8 @@ struct drm_bridge *msm_dsi_manager_bridge_init(u8 id)
bridge = _bridge->base;
bridge->funcs = _mgr_bridge_funcs;
+ drm_bridge_add(bridge);
+
ret = drm_bridge_attach(encoder, bridge, NULL, 0);
if (ret)
goto fail;
@@ -735,6 +737,7 @@ struct drm_connector *msm_dsi_manager_ext_bridge_init(u8 id)
void msm_dsi_manager_bridge_destroy(struct drm_bridge *bridge)
{
+ drm_bridge_remove(bridge);
}
int msm_dsi_manager_cmd_xfer(int id, const struct mipi_dsi_msg *msg)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 10ebe2089cb6..97c24010c4d1 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -15,6 +15,7 @@ void msm_hdmi_bridge_destroy(struct drm_bridge *bridge)
struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
msm_hdmi_hpd_disable(hdmi_bridge);
+ drm_bridge_remove(bridge);
}
static void msm_hdmi_power_on(struct drm_bridge *bridge)
@@ -349,6 +350,8 @@ struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi)
DRM_BRIDGE_OP_DETECT |
DRM_BRIDGE_OP_EDID;
+ drm_bridge_add(bridge);
+
ret = drm_bridge_attach(hdmi->encoder, bridge, NULL,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret)
goto fail;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 2905b82a9de3..d8f2c8838a7f 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -232,6 +232,9 @@ static int msm_drm_uninit(struct device *dev)
drm_mode_config_cleanup(ddev);
+ for (i = 0; i < priv->num_bridges; i++)
+ drm_bridge_remove(priv->bridges[i]);
+
Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly
Hi, On Fri, Apr 15, 2022 at 5:54 PM Dmitry Baryshkov wrote: > > >>> * They rely on there being exactly 1 AUX device, or we make it a rule > >>> that we wait for all AUX devices to probe (?) > >> > >> Is the backlight a separate device on an AUX bus? Judging from > >> drm_panel_dp_aux_backlight(), it isn't. I assumed that aux bus is just > >> a point-to-point bus, so there is always a single client. > > > > Define "device". ;-) > > "a device on the AUX bus" = the device, which lists dp_aux_bus_type as > dev->bus_type. Right. So I guess I was thinking that we _could_ have modeled the backlight as a device which lists dp_aux_bus_type as dev->bus_type. AKA: 1. We could have had a second node in the sc7180-trogdor-homestar device tree under the DP controller for the DP AUX backlight. 2. Instead of manually calling drm_panel_dp_aux_backlight() from panel-edp.c and panel-samsung-atna33xc20.c the backlight could have just probed on its own. 3. In the device tree, the panel could have had a link to the backlight's phandle which would have made the existing drm_panel_of_backlight() "just work" and we wouldn't have needed the manual call to drm_panel_dp_aux_backlight(). Oh. But. Maybe. Not. We couldn't really have done that because we would have been able to do the DP AUX transactions for the backlight without powering on the panel. So we couldn't really have probed them separately. OK, you guys win this round. ;-) > > It's a seperate "struct device" from a Linux point of view since it's > > a backlight class device. Certainly it's highly correlated to the > > display, but one can conceptually think of them as different devices, > > sorta. ;-) > > > > I actually dug a tiny bit more into the whole "touchscreen over aux". > > I guess DP 1.2 has a standard of "USB over DP AUX". No idea how that > > would be modeled, of course. > > Ugh. Do you have any details of the standard itself? Like how does it > looks like from the host point of view. And if the AUX is required to be > powered for this USB bus to work? > > In other words: if we were to model it at this moment, would it be the > child of the panel device (like backlight) or a separate device sitting > on the same AUX bus? I spent a bunch of time searching and I couldn't find more than a reference, like "hey we came up with this idea and we're gonna write down in the spec that you could totally do USB over the AUX channel, but u, we haven't actually implemented it yet". ;-) I certainly could be wrong, but all of the references I could find were distinctly lacking in details or pointers to other docs w/ more info. ...but while searching I _did_ find a lot of details (in the eDP 1.4 spec) about "Multi-touch over AUX". That looks like something that's actually more well thought out and maybe even implemented somewhere. >From what I can tell though, it looks as if it's the same thing as the backlight. In other words it's only available when the panel is powered. I don't think we want to do anything so drastic like moving the ownership of panel power to the AUX bus or anything, so I guess this means that: a) The panel driver will remain in charge of powering the panel (including anything on the DP AUX bus) on and off and getting the power sequencing right. b) That means that we can really think of the panel as the _only_ thing on the DP AUX bus. c) Anything else on the DP AUX bus will be underneath the panel driver. > > I guess the summary is that I'm OK w/ changing it to assume one device > > for now, but I'm still not sure it's compelling to move to normal > > callbacks. The API for callbacks is pretty much the same as the one I > > proposed and IMO leaving it the way it is (with an extra struct > > device) doesn't really add much complexity and has a few (small) nice > > benefits. > > I think Stephen didn't like too many similarities between > dp_aux_ep_client and dp_aux_ep_device. And I'd second him here. > > > >>> * We need to come up with a system for detecting when everything > >>> probes or is going to be removed, though that's probably not too hard. > >>> I guess the DP AUX bus could just replace the panel's probe function > >>> with its own and essentially "tail patch" it. I guess it could "head > >>> patch" the remove call? ...or is there some better way you were > >>> thinking of knowing when all our children probed? > >>> > >>> * The callback on the aux bus controller side would not be able to > >>> DEFER. In other words trying to acquire a reference to the panel can > >>> always be the last thing we do so we know there can be no reasons to > >>> defer after. This should be doable, but at least in the ps8640 case it > >>> will require changing the code a bit. I notice that today it actually > >>> tries to get the panel side _before_ it gets the MIPI side and it > >>> potentially can return -EPROBE_DEFER if it can't find the MIPI side. I > >>> guess I have a niggling feeling that we'll find some reason in the > >>> future that we can't
[PATCH v10] 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(>event_q) to probe()
-- move spin_lock_init(>event_lock) to probe()
Changes in v4:
-- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
Changes in v5:
-- cancel relocation of both dp_display_bind() and dp_display_unbind()
Changes in v6:
-- move empty event q to dp_event_thread_start()
Changes in v7:
-- call ktheread_stop() directly instead of dp_hpd_event_thread_stop() function
Changes in v8:
-- return error immediately if audio registration failed.
Changes in v9:
-- return error immediately if event thread create failed.
Changes in v10:
-- delete extra DRM_ERROR("failed to create DP event thread\n");
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
Reviewed-by: Stephen Boyd
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dp/dp_display.c | 39 +
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..4dc0758 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,8 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(>audio_comp);
}
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -266,9 +269,18 @@ static int dp_display_bind(struct device *dev, struct
device *master,
}
rc = dp_register_audio_driver(dev, dp->audio);
- if (rc)
+ if (rc) {
DRM_ERROR("Audio registration Dp failed\n");
+ goto end;
+ }
+ rc = dp_hpd_event_thread_start(dp);
+ if (rc) {
+ DRM_ERROR("Event thread create failed\n");
+ goto end;
+ }
+
+ return 0;
end:
return rc;
}
@@ -280,6 +292,11 @@ 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);
+
+ kthread_stop(dp->ev_tsk);
+
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1071,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,12 +1149,17 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
{
- init_waitqueue_head(_priv->event_q);
- spin_lock_init(_priv->event_lock);
+ /* set event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+ if (IS_ERR(dp_priv->ev_tsk))
+ return PTR_ERR(dp_priv->ev_tsk);
+
+ return 0;
}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
@@ -1266,7 +1288,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(>event_mutex);
+ init_waitqueue_head(>event_q);
+ spin_lock_init(>event_lock);
/* Store DP audio handle inside DP display */
dp->dp_display.dp_audio = dp->audio;
@@ -1441,8 +1466,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
Re: [PATCH v2 0/5] drm/ttm: Introduce TTM res manager debugfs helpers
On Tue, 2022-04-12 at 11:15 +0200, Christian König wrote: > > Hi Zack, > > Reviewed-by: Christian König for the > entire > series. > > One nit pick is that I want to get rid of using ttm_manager_type() in > drivers and use pointers to the members directly. But that's > something > for a later cleanup anyway. That sounds good to me. Let me know if you'd like me to hold off on pushing this until the ttm_manager_type changes are ready, otherwise I'll push it to drm-misc-next tomorrow. z
RE: [PATCH v2 2/2] drm: rcar-du: Add RZ/G2L DSI driver
Hi Laurent,
Thanks for the feedback.
> Subject: Re: [PATCH v2 2/2] drm: rcar-du: Add RZ/G2L DSI driver
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Mon, Mar 28, 2022 at 07:51:15AM +0100, Biju Das wrote:
> > This driver supports the MIPI DSI encoder found in the RZ/G2L SoC. It
> > currently supports DSI mode only.
> >
> > Signed-off-by: Biju Das
> > ---
> > v1->v2:
> > * Rework based on dt-binding change (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 "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:
> > *
> > ---
> > 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
> > 4 files changed, 844 insertions(+)
> > 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
> >
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> > b/drivers/gpu/drm/rcar-du/Kconfig index e40fb0c53f9b..83d7d28214a0
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -64,6 +64,14 @@ config DRM_RCAR_MIPI_DSI
> > help
> > Enable support for the R-Car Display Unit embedded MIPI DSI
> encoders.
> >
> > +config DRM_RZG2L_MIPI_DSI
> > + tristate "RZ/G2L MIPI DSI Encoder Support"
> > + depends on DRM_BRIDGE && OF
> > + depends on ARCH_RENESAS || COMPILE_TEST
> > + select DRM_MIPI_DSI
> > + help
> > + Enable support for the RZ/G2L Display Unit embedded MIPI DSI
> encoders.
> > +
> > config DRM_RCAR_VSP
> > bool "R-Car DU VSP Compositor Support" if ARM
> > default y if ARM64
> > diff --git a/drivers/gpu/drm/rcar-du/Makefile
> > b/drivers/gpu/drm/rcar-du/Makefile
> > index 01ba78e56b53..7fb4f8717fc4 100644
> > --- a/drivers/gpu/drm/rcar-du/Makefile
> > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > @@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_RCAR_LVDS) += rcar_lvds.o
> > obj-$(CONFIG_DRM_RCAR_MIPI_DSI)+= rcar_mipi_dsi.o
> >
> > obj-$(CONFIG_DRM_RZG2L_DU) += rzg2l-du-drm.o
> > +obj-$(CONFIG_DRM_RZG2L_MIPI_DSI) += rzg2l_mipi_dsi.o
> >
> > # 'remote-endpoint' is fixed up at run-time
> > DTC_FLAGS_rcar_du_of_lvds_r8a7790 += -Wno-graph_endpoint diff --git
> > a/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> > b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> > new file mode 100644
> > index ..3b785041ba5e
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> > @@ -0,0 +1,686 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * RZ/G2L MIPI DSI Encoder Driver
> > + *
> > + * Copyright (C) 2022 Renesas Electronics Corporation */ #include
> > + #include #include #include
> > + #include #include
> > +#include #include #include
> > + #include #include
> > + #include
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "rzg2l_mipi_dsi_regs.h"
> > +
> > +struct rzg2l_mipi_dsi {
> > + struct device *dev;
> > + void __iomem *mmio;
> > +
> > + struct reset_control *rstc;
> > + struct reset_control *arstc;
> > + struct reset_control *prstc;
> > +
> > + struct mipi_dsi_host host;
> > + struct drm_bridge bridge;
> > + struct drm_bridge *next_bridge;
> > +
> > + struct clk *vclk;
> > +
> > + enum mipi_dsi_pixel_format format;
> > + unsigned int num_data_lanes;
> > + unsigned int lanes;
> > + unsigned long mode_flags;
> > +};
> > +
> > +static inline struct rzg2l_mipi_dsi * bridge_to_rzg2l_mipi_dsi(struct
> > +drm_bridge *bridge) {
> > + return container_of(bridge, struct rzg2l_mipi_dsi, bridge); }
> > +
> > +static inline struct rzg2l_mipi_dsi * host_to_rzg2l_mipi_dsi(struct
> > +mipi_dsi_host *host) {
> > + return container_of(host, struct rzg2l_mipi_dsi, host); }
> > +
> > +static void rzg2l_mipi_dsi_phy_write(void __iomem *mem, u32 reg, u32
> > +data)
>
> You can pass a
RE: [PATCH v2 1/2] dt-bindings: display: bridge: Document RZ/G2L MIPI DSI TX bindings
Hi Laurent, Thanks for the feedback. > Subject: Re: [PATCH v2 1/2] dt-bindings: display: bridge: Document RZ/G2L > MIPI DSI TX bindings > > Hi Biju, > > Thank you for the patch. > > On Mon, Mar 28, 2022 at 07:49:30AM +0100, Biju Das wrote: > > The RZ/G2L MIPI DSI TX is embedded in the Renesas RZ/G2L family SoC's. > > It can operate in DSI mode, with up to four data lanes. > > > > Signed-off-by: Biju Das > > --- > > 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 > > RFC->v1: > > * Added a ref to dsi-controller.yaml. > > RFC:- > > * > > --- > > .../bindings/display/bridge/renesas,dsi.yaml | 175 > > ++ > > 1 file changed, 175 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml > > b/Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml > > new file mode 100644 > > index ..eebbf617c484 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi.yam > > +++ l > > @@ -0,0 +1,175 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > > > + > > +title: Renesas RZ/G2L MIPI DSI Encoder > > + > > +maintainers: > > + - Biju Das > > + > > +description: | > > + This binding describes the MIPI DSI encoder embedded in the Renesas > > + RZ/G2L alike family of SoC's. The encoder can operate in DSI mode, > > +with > > + up to four data lanes. > > + > > +allOf: > > + - $ref: /schemas/display/dsi-controller.yaml# > > + > > +properties: > > + compatible: > > +enum: > > + - renesas,rzg2l-mipi-dsi # RZ/G2L and RZ/V2L > > + > > + reg: > > +maxItems: 1 > > + > > + interrupts: > > +items: > > + - description: Sequence operation channel 0 interrupt > > + - description: Sequence operation channel 1 interrupt > > + - description: Video-Input operation channel 1 interrupt > > + - description: DSI Packet Receive interrupt > > + - description: DSI Fatal Error interrupt > > + - description: DSI D-PHY PPI interrupt > > + - description: Debug interrupt > > + > > + interrupt-names: > > +items: > > + - const: seq0 > > + - const: seq1 > > + - const: vin1 > > + - const: rcv > > + - const: ferr > > + - const: ppi > > + - const: debug > > + > > + clocks: > > +items: > > + - description: DSI D-PHY PLL multiplied clock > > + - description: DSI D-PHY system clock > > + - description: DSI AXI bus clock > > + - description: DSI Register access clock > > + - description: DSI Video clock > > + - description: DSI D-PHY Escape mode Receive clock > > Isn't this the escape mode *transmit* clock ? Yep, There is a mismatch between clk document and hardware manual. I have reported this issue to HW team for fixing the clk document. > > > + > > + clock-names: > > +items: > > + - const: pllclk > > + - const: sysclk > > + - const: aclk > > + - const: pclk > > + - const: vclk > > + - const: lpclk > > + > > + resets: > > +items: > > + - description: MIPI_DSI_CMN_RSTB > > + - description: MIPI_DSI_ARESET_N > > + - description: MIPI_DSI_PRESET_N > > + > > + reset-names: > > +items: > > + - const: rst > > + - const: arst > > + - const: prst > > + > > + power-domains: > > +maxItems: 1 > > + > > + ports: > > +$ref: /schemas/graph.yaml#/properties/ports > > + > > +properties: > > + port@0: > > +$ref: /schemas/graph.yaml#/properties/port > > +description: Parallel input port > > + > > + port@1: > > +$ref: /schemas/graph.yaml#/$defs/port-base > > +unevaluatedProperties: false > > +description: DSI output port > > + > > +properties: > > + endpoint: > > +$ref: /schemas/media/video-interfaces.yaml# > > +unevaluatedProperties: false > > + > > +properties: > > + data-lanes: > > +minItems: 1 > > +maxItems: 4 > > You should specify the acceptable values, especially given that the > hardware doesn't seem to support lane reordering. OK. > > > + > > +required: > > + - data-lanes > > + > > +required: > > + - port@0 > > + - port@1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - interrupt-names > > + - clocks > > + - clock-names > > + - resets > > + - reset-names > > + - power-domains > > + - ports > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > +#include > > +#include > > Could you please swap those two lines to get them sorted
[PATCH] drm/amd/display: add virtual_setup_stream_attribute decl to header
Smatch reports this issue virtual_link_hwss.c:32:6: warning: symbol 'virtual_setup_stream_attribute' was not declared. Should it be static? virtual_setup_stream_attribute is only used in virtual_link_hwss.c, but the other functions in the file are declared in the header file and used elsewhere. For consistency, add the virtual_setup_stream_attribute decl to virtual_link_hwss.h. Signed-off-by: Tom Rix --- drivers/gpu/drm/amd/display/dc/virtual/virtual_link_hwss.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/display/dc/virtual/virtual_link_hwss.h b/drivers/gpu/drm/amd/display/dc/virtual/virtual_link_hwss.h index e6bcb4bb0f3a..fbcbc5afb47d 100644 --- a/drivers/gpu/drm/amd/display/dc/virtual/virtual_link_hwss.h +++ b/drivers/gpu/drm/amd/display/dc/virtual/virtual_link_hwss.h @@ -28,6 +28,7 @@ #include "core_types.h" void virtual_setup_stream_encoder(struct pipe_ctx *pipe_ctx); +void virtual_setup_stream_attribute(struct pipe_ctx *pipe_ctx); void virtual_reset_stream_encoder(struct pipe_ctx *pipe_ctx); const struct link_hwss *get_virtual_link_hwss(void); -- 2.27.0
Re: [PATCH 04/11] drm/ttm: move default BO destructor into VMWGFX
On Tue, 2022-03-29 at 13:02 +0200, Christian König wrote:
> ⚠ External Email
>
> It's the only driver using this.
>
> Signed-off-by: Christian König
Looks good. A small suggestion underneath.
Reviewed-by: Zack Rusin
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 9 +
> drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 11 ++-
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> b/drivers/gpu/drm/ttm/ttm_bo.c
> index e5fd0f2c0299..7598d59423bf 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -44,12 +44,6 @@
>
> #include "ttm_module.h"
>
> -/* default destructor */
> -static void ttm_bo_default_destroy(struct ttm_buffer_object *bo)
> -{
> - kfree(bo);
> -}
> -
> static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
> struct ttm_placement
> *placement)
> {
> @@ -938,8 +932,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
> bool locked;
> int ret;
>
> - bo->destroy = destroy ? destroy : ttm_bo_default_destroy;
> -
> + bo->destroy = destroy;
> kref_init(>kref);
> INIT_LIST_HEAD(>ddestroy);
> bo->bdev = bdev;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 31aecc46624b..60dcc6a75248 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -378,6 +378,12 @@ void vmw_bo_bo_free(struct ttm_buffer_object
> *bo)
> kfree(vmw_bo);
> }
>
> +/* default destructor */
> +static void vmw_bo_default_destroy(struct ttm_buffer_object *bo)
> +{
> + kfree(bo);
> +}
> +
> /**
> * vmw_bo_create_kernel - Create a pinned BO for internal kernel
> use.
> *
> @@ -410,7 +416,7 @@ int vmw_bo_create_kernel(struct vmw_private
> *dev_priv, unsigned long size,
>
> ret = ttm_bo_init_reserved(_priv->bdev, bo, size,
> ttm_bo_type_kernel, placement, 0,
> - , NULL, NULL, NULL);
> + , NULL, NULL,
> vmw_bo_default_destroy);
> if (unlikely(ret))
> goto error_free;
>
> @@ -439,6 +445,9 @@ int vmw_bo_create(struct vmw_private *vmw,
> return -ENOMEM;
> }
>
> + if (!bo_free)
> + bo_free = vmw_bo_default_destroy;
> +
If you could change this to just BUG_ON(!bo_free) that'd be great.
bo_free == NULL should never happen here.
z
Re: [PATCH v4 09/15] drm/shmem-helper: Correct doc-comment of drm_gem_shmem_get_sg_table()
On 4/18/22 21:25, Thomas Zimmermann wrote:
> Hi
>
> Am 18.04.22 um 00:37 schrieb Dmitry Osipenko:
>> drm_gem_shmem_get_sg_table() never returns NULL on error, but a ERR_PTR.
>> Correct the doc comment which says that it returns NULL on error.
>>
>> Signed-off-by: Dmitry Osipenko
>
>
>> ---
>> drivers/gpu/drm/drm_gem_shmem_helper.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> index 8ad0e02991ca..30ee46348a99 100644
>> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
>> @@ -662,7 +662,7 @@ EXPORT_SYMBOL(drm_gem_shmem_print_info);
>> * drm_gem_shmem_get_pages_sgt() instead.
>> *
>> * Returns:
>> - * A pointer to the scatter/gather table of pinned pages or NULL on
>> failure.
>> + * A pointer to the scatter/gather table of pinned pages or errno on
>> failure.
>
> ', or an ERR_PTR()-encoded errno code on failure'
>
>> */
>> struct sg_table *drm_gem_shmem_get_sg_table(struct
>> drm_gem_shmem_object *shmem)
>> {
>> @@ -688,7 +688,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
>> * drm_gem_shmem_get_sg_table() should not be directly called by
>> drivers.
>> *
>> * Returns:
>> - * A pointer to the scatter/gather table of pinned pages or errno on
>> failure.
>> + * A pointer to the scatter/gather table of pinned pages
>> ERR_PTR()-encoded
>
> ', or an' before ERR_PTR
>
> With the improved grammar:
>
> Acked-by: Thomas Zimmermann
Thanks, something went wrong with these comments in this patch and I
haven't noticed that :)
[PATCH] drm/vmwgfx: Fix gem refcounting and memory evictions
From: Zack Rusin
The initial GEM port broke refcounting on shareable (prime) surfaces and
memory evictions. The prime surfaces broke because the parent surfaces
weren't increasing the ref count on GEM surfaces, which meant that
the memory backing textures could have been deleted while the texture
was still accessible. The evictions broke due to a typo, the code was
supposed to exit if the passed buffers were not vmw_buffer_object
not if they were. They're tied because the evictions depend on having
memory to actually evict.
This fixes crashes with XA state tracker which is used for xrender
acceleration on xf86-video-vmware and apps/tests which use a lot of
memory (a good test being the piglit's streaming-texture-leak)
Signed-off-by: Zack Rusin
Fixes: 8afa13a0583f ("drm/vmwgfx: Implement DRIVER_GEM")
Cc: # v5.17+
Reviewed-by: Maaz Mombasawala
Reviewed-by: Martin Krastev
---
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 38 +++--
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 8 ++
drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 7 -
3 files changed, 25 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 31aecc46624b..32326168b258 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -46,6 +46,21 @@ vmw_buffer_object(struct ttm_buffer_object *bo)
return container_of(bo, struct vmw_buffer_object, base);
}
+/**
+ * bo_is_vmw - check if the buffer object is a _buffer_object
+ * @bo: ttm buffer object to be checked
+ *
+ * Uses destroy function associated with the object to determine if this is
+ * a _buffer_object.
+ *
+ * Returns:
+ * true if the object is of _buffer_object type, false if not.
+ */
+static bool bo_is_vmw(struct ttm_buffer_object *bo)
+{
+ return bo->destroy == _bo_bo_free ||
+ bo->destroy == _gem_destroy;
+}
/**
* vmw_bo_pin_in_placement - Validate a buffer to placement.
@@ -798,7 +813,7 @@ int vmw_dumb_create(struct drm_file *file_priv,
void vmw_bo_swap_notify(struct ttm_buffer_object *bo)
{
/* Is @bo embedded in a struct vmw_buffer_object? */
- if (vmw_bo_is_vmw_bo(bo))
+ if (!bo_is_vmw(bo))
return;
/* Kill any cached kernel maps before swapout */
@@ -822,7 +837,7 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
struct vmw_buffer_object *vbo;
/* Make sure @bo is embedded in a struct vmw_buffer_object? */
- if (vmw_bo_is_vmw_bo(bo))
+ if (!bo_is_vmw(bo))
return;
vbo = container_of(bo, struct vmw_buffer_object, base);
@@ -843,22 +858,3 @@ void vmw_bo_move_notify(struct ttm_buffer_object *bo,
if (mem->mem_type != VMW_PL_MOB && bo->resource->mem_type == VMW_PL_MOB)
vmw_resource_unbind_list(vbo);
}
-
-/**
- * vmw_bo_is_vmw_bo - check if the buffer object is a _buffer_object
- * @bo: buffer object to be checked
- *
- * Uses destroy function associated with the object to determine if this is
- * a _buffer_object.
- *
- * Returns:
- * true if the object is of _buffer_object type, false if not.
- */
-bool vmw_bo_is_vmw_bo(struct ttm_buffer_object *bo)
-{
- if (bo->destroy == _bo_bo_free ||
- bo->destroy == _gem_destroy)
- return true;
-
- return false;
-}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 72a17618ba0a..0c12faa4e533 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1018,13 +1018,10 @@ static int vmw_driver_load(struct vmw_private
*dev_priv, u32 pci_id)
goto out_no_fman;
}
- drm_vma_offset_manager_init(_priv->vma_manager,
- DRM_FILE_PAGE_OFFSET_START,
- DRM_FILE_PAGE_OFFSET_SIZE);
ret = ttm_device_init(_priv->bdev, _bo_driver,
dev_priv->drm.dev,
dev_priv->drm.anon_inode->i_mapping,
- _priv->vma_manager,
+ dev_priv->drm.vma_offset_manager,
dev_priv->map_mode == vmw_dma_alloc_coherent,
false);
if (unlikely(ret != 0)) {
@@ -1195,7 +1192,6 @@ static void vmw_driver_unload(struct drm_device *dev)
vmw_devcaps_destroy(dev_priv);
vmw_vram_manager_fini(dev_priv);
ttm_device_fini(_priv->bdev);
- drm_vma_offset_manager_destroy(_priv->vma_manager);
vmw_release_device_late(dev_priv);
vmw_fence_manager_takedown(dev_priv->fman);
if (dev_priv->capabilities & SVGA_CAP_IRQMASK)
@@ -1419,7 +1415,7 @@ vmw_get_unmapped_area(struct file *file, unsigned long
uaddr,
struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
return drm_get_unmapped_area(file, uaddr, len, pgoff, flags,
-
Re: [PATCH v9] drm/msm/dp: stop event kernel thread when DP unbind
Quoting Kuogee Hsieh (2022-04-15 16:47:25)
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 01453db..5b289b9 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -266,9 +269,18 @@ static int dp_display_bind(struct device *dev, struct
> device *master,
> }
>
> rc = dp_register_audio_driver(dev, dp->audio);
> - if (rc)
> + if (rc) {
> DRM_ERROR("Audio registration Dp failed\n");
> + goto end;
> + }
>
> + rc = dp_hpd_event_thread_start(dp);
> + if (rc) {
> + DRM_ERROR("Event thread create failed\n");
One thread DRM_ERROR()
> + goto end;
> + }
> +
> + return 0;
> end:
> return rc;
> }
> @@ -1132,12 +1149,19 @@ static int hpd_event_thread(void *data)
> return 0;
> }
>
> -static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
> +static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
> {
> - init_waitqueue_head(_priv->event_q);
> - spin_lock_init(_priv->event_lock);
> + /* set event q to empty */
> + dp_priv->event_gndx = 0;
> + dp_priv->event_pndx = 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");
And another thread creation DRM_ERROR(). Can we just have one please
instead of two lines for something that probably never happens?
> + return PTR_ERR(dp_priv->ev_tsk);
> + }
>
> - kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
> + return 0;
> }
>
> static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
Re: [PATCH v4 10/15] drm/shmem-helper: Take reservation lock instead of drm_gem_shmem locks
Hello,
On 4/18/22 21:38, Thomas Zimmermann wrote:
> Hi
>
> Am 18.04.22 um 00:37 schrieb Dmitry Osipenko:
>> Replace drm_gem_shmem locks with the reservation lock to make GEM
>> lockings more consistent.
>>
>> Previously drm_gem_shmem_vmap() and drm_gem_shmem_get_pages() were
>> protected by separate locks, now it's the same lock, but it doesn't
>> make any difference for the current GEM SHMEM users. Only Panfrost
>> and Lima drivers use vmap() and they do it in the slow code paths,
>> hence there was no practical justification for the usage of separate
>> lock in the vmap().
>>
>> Suggested-by: Daniel Vetter
>> Signed-off-by: Dmitry Osipenko
>> ---
...
>> @@ -310,7 +306,7 @@ static int drm_gem_shmem_vmap_locked(struct
>> drm_gem_shmem_object *shmem,
>> } else {
>> pgprot_t prot = PAGE_KERNEL;
>> - ret = drm_gem_shmem_get_pages(shmem);
>> + ret = drm_gem_shmem_get_pages_locked(shmem);
>> if (ret)
>> goto err_zero_use;
>> @@ -360,11 +356,11 @@ int drm_gem_shmem_vmap(struct
>> drm_gem_shmem_object *shmem,
>> {
>> int ret;
>> - ret = mutex_lock_interruptible(>vmap_lock);
>> + ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
>> if (ret)
>> return ret;
>> ret = drm_gem_shmem_vmap_locked(shmem, map);
>
> Within drm_gem_shmem_vmap_locked(), there's a call to dma_buf_vmap() for
> imported pages. If the exporter side also holds/acquires the same
> reservation lock as our object, the whole thing can deadlock. We cannot
> move dma_buf_vmap() out of the CS, because we still need to increment
> the reference counter. I honestly don't know how to easily fix this
> problem. There's a TODO item about replacing these locks at [1]. As
> Daniel suggested this patch, we should talk to him about the issue.
>
> Best regards
> Thomas
>
> [1]
> https://www.kernel.org/doc/html/latest/gpu/todo.html#move-buffer-object-locking-to-dma-resv-lock
Indeed, good catch! Perhaps we could simply use a separate lock for the
vmapping of the *imported* GEMs? The vmap_use_count is used only by
vmap/vunmap, so it doesn't matter which lock is used by these functions
in the case of imported GEMs since we only need to protect the
vmap_use_count.
Re: [PATCH] drm/nouveau/gr/gf100-: change gf108_gr_fwif from global to static
Reviewed-by: Lyude Paul
Will push to the appropriate branch in a moment
On Mon, 2022-04-18 at 11:28 -0400, Tom Rix wrote:
> Smatch reports this issue
> gf108.c:147:1: warning: symbol 'gf108_gr_fwif'
> was not declared. Should it be static?
>
> gf108_gr_fwif is only used in gf108.c. Single
> file variables should not be global so change
> gf108_gr_fwif's storage-class specifier to static.
>
> Signed-off-by: Tom Rix
> ---
> drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c
> index 030640bb3dca..ab3760e804b8 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c
> @@ -143,7 +143,7 @@ gf108_gr = {
> }
> };
>
> -const struct gf100_gr_fwif
> +static const struct gf100_gr_fwif
> gf108_gr_fwif[] = {
> { -1, gf100_gr_load, _gr },
> { -1, gf100_gr_nofw, _gr },
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [PATCH] drm/nouveau: change base917c_format from global to static
Reviewed-by: Lyude Paul
Will push this to the appropriate branch in a little bit
On Mon, 2022-04-18 at 10:18 -0400, Tom Rix wrote:
> Smatch reports this issue
> base917c.c:26:1: warning: symbol 'base917c_format'
> was not declared. Should it be static?
>
> base917c_format is only used in base917.c. Single
> file variables should not be global so change
> base917c_format's storage-class specifier to static.
>
> Signed-off-by: Tom Rix
> ---
> drivers/gpu/drm/nouveau/dispnv50/base917c.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/base917c.c
> b/drivers/gpu/drm/nouveau/dispnv50/base917c.c
> index a1baed4fe0e9..ca260509a4f1 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/base917c.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/base917c.c
> @@ -22,7 +22,7 @@
> #include "base.h"
> #include "atom.h"
>
> -const u32
> +static const u32
> base917c_format[] = {
> DRM_FORMAT_C8,
> DRM_FORMAT_XRGB,
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [PATCH v4 10/15] drm/shmem-helper: Take reservation lock instead of drm_gem_shmem locks
Hi
Am 18.04.22 um 00:37 schrieb Dmitry Osipenko:
Replace drm_gem_shmem locks with the reservation lock to make GEM
lockings more consistent.
Previously drm_gem_shmem_vmap() and drm_gem_shmem_get_pages() were
protected by separate locks, now it's the same lock, but it doesn't
make any difference for the current GEM SHMEM users. Only Panfrost
and Lima drivers use vmap() and they do it in the slow code paths,
hence there was no practical justification for the usage of separate
lock in the vmap().
Suggested-by: Daniel Vetter
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 38 -
drivers/gpu/drm/lima/lima_gem.c | 8 +++---
drivers/gpu/drm/panfrost/panfrost_mmu.c | 15 ++
include/drm/drm_gem_shmem_helper.h | 10 ---
4 files changed, 31 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 30ee46348a99..3ecef571eff3 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -86,8 +86,6 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size,
bool private)
if (ret)
goto err_release;
- mutex_init(>pages_lock);
- mutex_init(>vmap_lock);
INIT_LIST_HEAD(>madv_list);
if (!private) {
@@ -157,8 +155,6 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
WARN_ON(shmem->pages_use_count);
drm_gem_object_release(obj);
- mutex_destroy(>pages_lock);
- mutex_destroy(>vmap_lock);
kfree(shmem);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
@@ -209,11 +205,11 @@ int drm_gem_shmem_get_pages(struct drm_gem_shmem_object
*shmem)
WARN_ON(shmem->base.import_attach);
- ret = mutex_lock_interruptible(>pages_lock);
+ ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
if (ret)
return ret;
ret = drm_gem_shmem_get_pages_locked(shmem);
- mutex_unlock(>pages_lock);
+ dma_resv_unlock(shmem->base.resv);
return ret;
}
@@ -248,9 +244,9 @@ static void drm_gem_shmem_put_pages_locked(struct
drm_gem_shmem_object *shmem)
*/
void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
{
- mutex_lock(>pages_lock);
+ dma_resv_lock(shmem->base.resv, NULL);
drm_gem_shmem_put_pages_locked(shmem);
- mutex_unlock(>pages_lock);
+ dma_resv_unlock(shmem->base.resv);
}
EXPORT_SYMBOL(drm_gem_shmem_put_pages);
@@ -310,7 +306,7 @@ static int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
} else {
pgprot_t prot = PAGE_KERNEL;
- ret = drm_gem_shmem_get_pages(shmem);
+ ret = drm_gem_shmem_get_pages_locked(shmem);
if (ret)
goto err_zero_use;
@@ -360,11 +356,11 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
{
int ret;
- ret = mutex_lock_interruptible(>vmap_lock);
+ ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
if (ret)
return ret;
ret = drm_gem_shmem_vmap_locked(shmem, map);
Within drm_gem_shmem_vmap_locked(), there's a call to dma_buf_vmap() for
imported pages. If the exporter side also holds/acquires the same
reservation lock as our object, the whole thing can deadlock. We cannot
move dma_buf_vmap() out of the CS, because we still need to increment
the reference counter. I honestly don't know how to easily fix this
problem. There's a TODO item about replacing these locks at [1]. As
Daniel suggested this patch, we should talk to him about the issue.
Best regards
Thomas
[1]
https://www.kernel.org/doc/html/latest/gpu/todo.html#move-buffer-object-locking-to-dma-resv-lock
- mutex_unlock(>vmap_lock);
+ dma_resv_unlock(shmem->base.resv);
return ret;
}
@@ -385,7 +381,7 @@ static void drm_gem_shmem_vunmap_locked(struct
drm_gem_shmem_object *shmem,
dma_buf_vunmap(obj->import_attach->dmabuf, map);
} else {
vunmap(shmem->vaddr);
- drm_gem_shmem_put_pages(shmem);
+ drm_gem_shmem_put_pages_locked(shmem);
}
shmem->vaddr = NULL;
@@ -406,9 +402,11 @@ static void drm_gem_shmem_vunmap_locked(struct
drm_gem_shmem_object *shmem,
void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem,
struct iosys_map *map)
{
- mutex_lock(>vmap_lock);
+ dma_resv_lock(shmem->base.resv, NULL);
drm_gem_shmem_vunmap_locked(shmem, map);
- mutex_unlock(>vmap_lock);
+ dma_resv_unlock(shmem->base.resv);
+
+ drm_gem_shmem_update_purgeable_status(shmem);
}
EXPORT_SYMBOL(drm_gem_shmem_vunmap);
@@ -442,14 +440,14 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
*/
int drm_gem_shmem_madvise(struct drm_gem_shmem_object *shmem, int madv)
{
- mutex_lock(>pages_lock);
+
Re: [PATCH v4 09/15] drm/shmem-helper: Correct doc-comment of drm_gem_shmem_get_sg_table()
Hi
Am 18.04.22 um 00:37 schrieb Dmitry Osipenko:
drm_gem_shmem_get_sg_table() never returns NULL on error, but a ERR_PTR.
Correct the doc comment which says that it returns NULL on error.
Signed-off-by: Dmitry Osipenko
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 8ad0e02991ca..30ee46348a99 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -662,7 +662,7 @@ EXPORT_SYMBOL(drm_gem_shmem_print_info);
* drm_gem_shmem_get_pages_sgt() instead.
*
* Returns:
- * A pointer to the scatter/gather table of pinned pages or NULL on failure.
+ * A pointer to the scatter/gather table of pinned pages or errno on failure.
', or an ERR_PTR()-encoded errno code on failure'
*/
struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_shmem_object
*shmem)
{
@@ -688,7 +688,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
* drm_gem_shmem_get_sg_table() should not be directly called by drivers.
*
* Returns:
- * A pointer to the scatter/gather table of pinned pages or errno on failure.
+ * A pointer to the scatter/gather table of pinned pages ERR_PTR()-encoded
', or an' before ERR_PTR
With the improved grammar:
Acked-by: Thomas Zimmermann
+ * error code on failure.
*/
struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_shmem_object
*shmem)
{
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH 3/8] drm/display: Introduce a DRM display-helper module
Hi Jani Am 07.04.22 um 10:45 schrieb Jani Nikula: ... I think another idea that could work is to use an intermediate symbol. For DP, drivers would select the tristate DP_HELPER, which in turn selects tristate DISPLAY_HELPER and boolean DISPLAY_DP_HELPER. But this would require a 'useless' symbol DP_HELPER only for convenience. It's an even less optimal solution, it seems. Documentation/kbuild/kconfig-language.rst: Note: select should be used with care. select will force a symbol to a value without visiting the dependencies. By abusing select you are able to select a symbol FOO even if FOO depends on BAR that is not set. --> In general use select only for non-visible symbols --> (no prompts anywhere) and for symbols with no dependencies. That will limit the usefulness but on the other hand avoid the illegal configurations all over. Most of the difficult Kconfig issues I've encountered over the years come from not following the above two rules. People break those rules for "convenience", causing a lot of inconvenience down the line. I have meanwhile updated the patchset and all new boolean options are internal. No select will be performed on 'visible' symbols. So it should be fine. Best regards Thomas BR, Jani. Best regards Thomas -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 1/2] of: Create platform devices for OF framebuffers
Hi
Am 13.04.22 um 14:51 schrieb Rob Herring:
...
+
/**
* of_platform_populate() - Populate platform_devices from device tree data
* @root: parent of the first level to probe or NULL for the root of the tree
@@ -541,9 +595,7 @@ static int __init of_platform_default_populate_init(void)
of_node_put(node);
}
- node = of_get_compatible_child(of_chosen, "simple-framebuffer");
- of_platform_device_create(node, NULL, NULL);
- of_node_put(node);
+ of_platform_populate_framebuffers();
/* Populate everything else. */
of_platform_default_populate(NULL, NULL, NULL);
I'm pretty sure it's just this call that's the problem for PPC though
none of the above existed when adding this caused a regression. Can we
remove the ifdef and just make this call conditional on
!IS_ENABLED(CONFIG_PPC).
That didn't work. The boot process stops at some point. I'll send you an
updated patch that covers most of the function with IS_ENABLED(CONFIG_PPC)
Best regards
Thomas
@@ -551,6 +603,20 @@ static int __init of_platform_default_populate_init(void)
return 0;
}
arch_initcall_sync(of_platform_default_populate_init);
+#else
+static int __init of_platform_default_populate_init(void)
+{
+ device_links_supplier_sync_state_pause();
+
+ if (!of_have_populated_dt())
+ return -ENODEV;
+
+ of_platform_populate_framebuffers();
+
+ return 0;
+}
+arch_initcall_sync(of_platform_default_populate_init);
+#endif
static int __init of_platform_sync_state_init(void)
{
@@ -558,7 +624,6 @@ static int __init of_platform_sync_state_init(void)
return 0;
}
late_initcall_sync(of_platform_sync_state_init);
-#endif
int of_platform_device_destroy(struct device *dev, void *data)
{
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
OpenPGP_signature
Description: OpenPGP digital signature
Re: [RFC PATCH 4/6] drm/panel-edp: Take advantage of is_hpd_asserted() in struct drm_dp_aux
Hi,
On Fri, Apr 15, 2022 at 5:14 PM Dmitry Baryshkov
wrote:
>
> On 16/04/2022 03:12, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, Apr 15, 2022 at 3:12 PM Dmitry Baryshkov
> > wrote:
> >>
> >> On Sat, 16 Apr 2022 at 00:17, Doug Anderson wrote:
> >>>
> >>> Hi,
> >>>
> >>> On Thu, Apr 14, 2022 at 5:51 PM Dmitry Baryshkov
> >>> wrote:
>
> On 09/04/2022 05:36, Douglas Anderson wrote:
> > Let's add support for being able to read the HPD pin even if it's
> > hooked directly to the controller. This will allow us to get more
> > accurate delays also lets us take away the waiting in the AUX transfer
> > functions of the eDP controller drivers.
> >
> > Signed-off-by: Douglas Anderson
> > ---
> >
> >drivers/gpu/drm/panel/panel-edp.c | 37
> > ++-
> >1 file changed, 31 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c
> > b/drivers/gpu/drm/panel/panel-edp.c
> > index 1732b4f56e38..4a143eb9544b 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -417,6 +417,19 @@ static int panel_edp_get_hpd_gpio(struct device
> > *dev, struct panel_edp *p)
> >return 0;
> >}
> >
> > +static bool panel_edp_can_read_hpd(struct panel_edp *p)
> > +{
> > + return !p->no_hpd && (p->hpd_gpio || (p->aux &&
> > p->aux->is_hpd_asserted));
> > +}
> > +
> > +static bool panel_edp_read_hpd(struct panel_edp *p)
> > +{
> > + if (p->hpd_gpio)
> > + return gpiod_get_value_cansleep(p->hpd_gpio);
> > +
> > + return p->aux->is_hpd_asserted(p->aux);
> > +}
> > +
> >static int panel_edp_prepare_once(struct panel_edp *p)
> >{
> >struct device *dev = p->base.dev;
> > @@ -441,13 +454,21 @@ static int panel_edp_prepare_once(struct
> > panel_edp *p)
> >if (delay)
> >msleep(delay);
> >
> > - if (p->hpd_gpio) {
> > + if (panel_edp_can_read_hpd(p)) {
> >if (p->desc->delay.hpd_absent)
> >hpd_wait_us = p->desc->delay.hpd_absent * 1000UL;
> >else
> >hpd_wait_us = 200;
> >
> > - err = readx_poll_timeout(gpiod_get_value_cansleep,
> > p->hpd_gpio,
> > + /*
> > + * Extra max delay, mostly to account for ps8640. ps8640
> > + * is crazy and the bridge chip driver itself has over
> > 200 ms
> > + * of delay if it needs to do the pm_runtime resume of the
> > + * bridge chip to read the HPD.
> > + */
> > + hpd_wait_us += 300;
>
> I think this should come in a separate commit and ideally this should be
> configurable somehow. Other hosts wouldn't need such 'additional' delay.
>
> With this change removed:
>
> Reviewed-by: Dmitry Baryshkov
> >>>
> >>> What would you think about changing the API slightly? Instead of
> >>> is_hpd_asserted(), we change it to wait_hpd_asserted() and it takes a
> >>> timeout in microseconds. If you pass 0 for the timeout the function is
> >>> defined to behave the same as is_hpd_asserted() today--AKA a single
> >>> poll of the line.
> >>
> >> This might work. Can you check it, please?
> >
> > Cool. I'll spin with this. Hopefully early next week unless my inbox
> > blows up. ...or my main PC's SSD like happened this week. ;-)
> >
> >
> >> BTW: are these changes dependent on the first part of the patchset? It
> >> might be worth splitting the patchset into two parts.
> >
> > Definitely not. As per the cover letter, this is two series jammed
> > into one. I'm happy to split them up. The 2nd half seems much less
> > controversial.
>
> Great, let's get it in then. As you have time.
Breadcrumbs: I've posted this as:
https://lore.kernel.org/r/20220418171757.2282651-1-diand...@chromium.org
-Doug
[PATCH v3 4/4] drm/bridge: parade-ps8640: Provide wait_hpd_asserted() in struct drm_dp_aux
This implements the callback added by the patch ("drm/dp: Add
wait_hpd_asserted() callback to struct drm_dp_aux").
With this change and all the two "DP AUX Endpoint" drivers changed to
use wait_hpd_asserted(), we no longer need to have an long delay in
the AUX transfer function. It's up to the panel code to make sure that
the panel is powered now. If someone tried to call the aux transfer
function without making sure the panel is powered we'll just get a
normal transfer failure.
We'll still keep the wait for HPD in the pre_enable() function. Though
it's probably not actually needed there, this driver is used in the
old mode (pre-DP AUX Endpoints) and it may be important for those
cases. If nothing else, it shouldn't cause any big problems.
Signed-off-by: Douglas Anderson
---
(no changes since v2)
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
drivers/gpu/drm/bridge/parade-ps8640.c | 34 --
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 9766cbbd62ad..2f19a8c89880 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -168,23 +168,30 @@ static bool ps8640_of_panel_on_aux_bus(struct device *dev)
return true;
}
-static int ps8640_ensure_hpd(struct ps8640 *ps_bridge)
+static int _ps8640_wait_hpd_asserted(struct ps8640 *ps_bridge, unsigned long
wait_us)
{
struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
- struct device *dev = _bridge->page[PAGE2_TOP_CNTL]->dev;
int status;
- int ret;
/*
* Apparently something about the firmware in the chip signals that
* HPD goes high by reporting GPIO9 as high (even though HPD isn't
* actually connected to GPIO9).
*/
- ret = regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
- status & PS_GPIO9, 20 * 1000, 200 *
1000);
+ return regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
+ status & PS_GPIO9, wait_us / 10,
wait_us);
+}
- if (ret < 0)
- dev_warn(dev, "HPD didn't go high: %d\n", ret);
+static int ps8640_wait_hpd_asserted(struct drm_dp_aux *aux, unsigned long
wait_us)
+{
+ struct ps8640 *ps_bridge = aux_to_ps8640(aux);
+ struct device *dev = _bridge->page[PAGE0_DP_CNTL]->dev;
+ int ret;
+
+ pm_runtime_get_sync(dev);
+ ret = _ps8640_wait_hpd_asserted(ps_bridge, wait_us);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
return ret;
}
@@ -323,9 +330,7 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
int ret;
pm_runtime_get_sync(dev);
- ret = ps8640_ensure_hpd(ps_bridge);
- if (!ret)
- ret = ps8640_aux_transfer_msg(aux, msg);
+ ret = ps8640_aux_transfer_msg(aux, msg);
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
@@ -369,8 +374,8 @@ static int __maybe_unused ps8640_resume(struct device *dev)
* Mystery 200 ms delay for the "MCU to be ready". It's unclear if
* this is truly necessary since the MCU will already signal that
* things are "good to go" by signaling HPD on "gpio 9". See
-* ps8640_ensure_hpd(). For now we'll keep this mystery delay just in
-* case.
+* _ps8640_wait_hpd_asserted(). For now we'll keep this mystery delay
+* just in case.
*/
msleep(200);
@@ -406,7 +411,9 @@ static void ps8640_pre_enable(struct drm_bridge *bridge)
int ret;
pm_runtime_get_sync(dev);
- ps8640_ensure_hpd(ps_bridge);
+ ret = _ps8640_wait_hpd_asserted(ps_bridge, 200 * 1000);
+ if (ret < 0)
+ dev_warn(dev, "HPD didn't go high: %d\n", ret);
/*
* The Manufacturer Command Set (MCS) is a device dependent interface
@@ -652,6 +659,7 @@ static int ps8640_probe(struct i2c_client *client)
ps_bridge->aux.name = "parade-ps8640-aux";
ps_bridge->aux.dev = dev;
ps_bridge->aux.transfer = ps8640_aux_transfer;
+ ps_bridge->aux.wait_hpd_asserted = ps8640_wait_hpd_asserted;
drm_dp_aux_init(_bridge->aux);
pm_runtime_enable(dev);
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH v3 3/4] drm/panel: atna33xc20: Take advantage of wait_hpd_asserted() in struct drm_dp_aux
Let's add support for being able to read the HPD pin even if it's
hooked directly to the controller. This will let us take away the
waiting in the AUX transfer functions of the eDP controller drivers.
Signed-off-by: Douglas Anderson
---
Changes in v3:
- Don't check "hpd_asserted" boolean when unset.
- Handle errors from gpiod_get_value_cansleep() properly.
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
.../gpu/drm/panel/panel-samsung-atna33xc20.c | 41 +--
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
index 20666b6217e7..5ef1b4032c56 100644
--- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
+++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
@@ -19,6 +19,10 @@
#include
#include
+/* T3 VCC to HPD high is max 200 ms */
+#define HPD_MAX_MS 200
+#define HPD_MAX_US (HPD_MAX_MS * 1000)
+
struct atana33xc20_panel {
struct drm_panel base;
bool prepared;
@@ -30,6 +34,7 @@ struct atana33xc20_panel {
struct regulator *supply;
struct gpio_desc *el_on3_gpio;
+ struct drm_dp_aux *aux;
struct edid *edid;
@@ -79,7 +84,7 @@ static int atana33xc20_suspend(struct device *dev)
static int atana33xc20_resume(struct device *dev)
{
struct atana33xc20_panel *p = dev_get_drvdata(dev);
- bool hpd_asserted = false;
+ int hpd_asserted;
int ret;
/* T12 (Power off time) is min 500 ms */
@@ -91,20 +96,28 @@ static int atana33xc20_resume(struct device *dev)
p->powered_on_time = ktime_get();
/*
-* Handle HPD. Note: if HPD is hooked up to a dedicated pin on the
-* eDP controller then "no_hpd" will be false _and_ "hpd_gpio" will be
-* NULL. It's up to the controller driver to wait for HPD after
-* preparing the panel in that case.
+* Note that it's possible that no_hpd is false, hpd_gpio is
+* NULL, and wait_hpd_asserted is NULL. This is because
+* wait_hpd_asserted() is optional even if HPD is hooked up to
+* a dedicated pin on the eDP controller. In this case we just
+* assume that the controller driver will wait for HPD at the
+* right times.
*/
if (p->no_hpd) {
- /* T3 VCC to HPD high is max 200 ms */
- msleep(200);
- } else if (p->hpd_gpio) {
- ret = readx_poll_timeout(gpiod_get_value_cansleep, p->hpd_gpio,
-hpd_asserted, hpd_asserted,
-1000, 20);
- if (!hpd_asserted)
- dev_warn(dev, "Timeout waiting for HPD\n");
+ msleep(HPD_MAX_MS);
+ } else {
+ if (p->hpd_gpio) {
+ ret = readx_poll_timeout(gpiod_get_value_cansleep,
+p->hpd_gpio, hpd_asserted,
+hpd_asserted, 1000,
HPD_MAX_US);
+ if (hpd_asserted < 0)
+ ret = hpd_asserted;
+ } else if (p->aux->wait_hpd_asserted) {
+ ret = p->aux->wait_hpd_asserted(p->aux, HPD_MAX_US);
+ }
+
+ if (ret)
+ dev_warn(dev, "Error waiting for HPD: %d\n", ret);
}
return 0;
@@ -263,6 +276,8 @@ static int atana33xc20_probe(struct dp_aux_ep_device
*aux_ep)
return -ENOMEM;
dev_set_drvdata(dev, panel);
+ panel->aux = aux_ep->aux;
+
panel->supply = devm_regulator_get(dev, "power");
if (IS_ERR(panel->supply))
return dev_err_probe(dev, PTR_ERR(panel->supply),
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH v3 1/4] drm/dp: Add wait_hpd_asserted() callback to struct drm_dp_aux
Sometimes it's useful for users of the DP AUX bus (like panels) to be
able to poll HPD. Let's add a callback that allows DP AUX busses
drivers to provide this.
Suggested-by: Dmitry Baryshkov
Signed-off-by: Douglas Anderson
---
Left Dmitry's Reviewed-by tag off since patch changed enough.
(no changes since v2)
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
include/drm/dp/drm_dp_helper.h | 26 ++
1 file changed, 26 insertions(+)
diff --git a/include/drm/dp/drm_dp_helper.h b/include/drm/dp/drm_dp_helper.h
index 53d1e722f4de..0940c415db8c 100644
--- a/include/drm/dp/drm_dp_helper.h
+++ b/include/drm/dp/drm_dp_helper.h
@@ -2035,6 +2035,32 @@ struct drm_dp_aux {
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
+ /**
+* @wait_hpd_asserted: wait for HPD to be asserted
+*
+* This is mainly useful for eDP panels drivers to wait for an eDP
+* panel to finish powering on. This is an optional function.
+*
+* This function will efficiently wait for up to `wait_us` microseconds
+* for HPD to be asserted and might sleep.
+*
+* This function returns 0 if HPD was asserted or -ETIMEDOUT if time
+* expired and HPD wasn't asserted. This function should not print
+* timeout errors to the log.
+*
+* The semantics of this function are designed to match the
+* readx_poll_timeout() function. That means a `wait_us` of 0 means
+* to wait forever. If you want to do a quick poll you could pass 1
+* for `wait_us`.
+*
+* NOTE: this function specifically reports the state of the HPD pin
+* that's associated with the DP AUX channel. This is different from
+* the HPD concept in much of the rest of DRM which is more about
+* physical presence of a display. For eDP, for instance, a display is
+* assumed always present even if the HPD pin is deasserted.
+*/
+ int (*wait_hpd_asserted)(struct drm_dp_aux *aux, unsigned long wait_us);
+
/**
* @i2c_nack_count: Counts I2C NACKs, used for DP validation.
*/
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH v3 2/4] drm/panel-edp: Take advantage of wait_hpd_asserted() in struct drm_dp_aux
Let's add support for being able to read the HPD pin even if it's
hooked directly to the controller. This will allow us to get more
accurate delays also lets us take away the waiting in the AUX transfer
functions of the eDP controller drivers.
Signed-off-by: Douglas Anderson
---
(no changes since v2)
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
drivers/gpu/drm/panel/panel-edp.c | 33 +--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 1732b4f56e38..086e0bf52fb9 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -417,6 +417,11 @@ static int panel_edp_get_hpd_gpio(struct device *dev,
struct panel_edp *p)
return 0;
}
+static bool panel_edp_can_read_hpd(struct panel_edp *p)
+{
+ return !p->no_hpd && (p->hpd_gpio || (p->aux &&
p->aux->wait_hpd_asserted));
+}
+
static int panel_edp_prepare_once(struct panel_edp *p)
{
struct device *dev = p->base.dev;
@@ -441,17 +446,21 @@ static int panel_edp_prepare_once(struct panel_edp *p)
if (delay)
msleep(delay);
- if (p->hpd_gpio) {
+ if (panel_edp_can_read_hpd(p)) {
if (p->desc->delay.hpd_absent)
hpd_wait_us = p->desc->delay.hpd_absent * 1000UL;
else
hpd_wait_us = 200;
- err = readx_poll_timeout(gpiod_get_value_cansleep, p->hpd_gpio,
-hpd_asserted, hpd_asserted,
-1000, hpd_wait_us);
- if (hpd_asserted < 0)
- err = hpd_asserted;
+ if (p->hpd_gpio) {
+ err = readx_poll_timeout(gpiod_get_value_cansleep,
+p->hpd_gpio, hpd_asserted,
+hpd_asserted, 1000,
hpd_wait_us);
+ if (hpd_asserted < 0)
+ err = hpd_asserted;
+ } else {
+ err = p->aux->wait_hpd_asserted(p->aux, hpd_wait_us);
+ }
if (err) {
if (err != -ETIMEDOUT)
@@ -532,18 +541,22 @@ static int panel_edp_enable(struct drm_panel *panel)
/*
* If there is a "prepare_to_enable" delay then that's supposed to be
* the delay from HPD going high until we can turn the backlight on.
-* However, we can only count this if HPD is handled by the panel
-* driver, not if it goes to a dedicated pin on the controller.
+* However, we can only count this if HPD is readable by the panel
+* driver.
+*
* If we aren't handling the HPD pin ourselves then the best we
* can do is assume that HPD went high immediately before we were
-* called (and link training took zero time).
+* called (and link training took zero time). Note that "no-hpd"
+* actually counts as handling HPD ourselves since we're doing the
+* worst case delay (in prepare) ourselves.
*
* NOTE: if we ever end up in this "if" statement then we're
* guaranteed that the panel_edp_wait() call below will do no delay.
* It already handles that case, though, so we don't need any special
* code for it.
*/
- if (p->desc->delay.prepare_to_enable && !p->hpd_gpio && !p->no_hpd)
+ if (p->desc->delay.prepare_to_enable &&
+ !panel_edp_can_read_hpd(p) && !p->no_hpd)
delay = max(delay, p->desc->delay.prepare_to_enable);
if (delay)
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH v3 0/4] drm/dp: Introduce wait_hpd_asserted() for the DP AUX bus
This is the 2nd four patches from my RFC series ("drm/dp: Improvements
for DP AUX channel") [1]. I've broken the series in two so we can make
progress on the two halves separately.
v2 of this series changes to add wait_hpd_asserted() instead of
is_hpd_asserted(). This allows us to move the extra delay needed for
ps8640 into the ps8640 driver itself.
The idea for this series came up during the review process of
Sankeerth's series trying to add eDP for Qualcomm SoCs [2].
This _doesn't_ attempt to fix the Analogix driver. If this works out,
ideally someone can post a patch up to do that.
[1] https://lore.kernel.org/r/20220409023628.2104952-1-diand...@chromium.org/
[2]
https://lore.kernel.org/r/1648656179-10347-2-git-send-email-quic_sbill...@quicinc.com/
Changes in v3:
- Don't check "hpd_asserted" boolean when unset.
- Handle errors from gpiod_get_value_cansleep() properly.
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
Douglas Anderson (4):
drm/dp: Add wait_hpd_asserted() callback to struct drm_dp_aux
drm/panel-edp: Take advantage of wait_hpd_asserted() in struct
drm_dp_aux
drm/panel: atna33xc20: Take advantage of wait_hpd_asserted() in struct
drm_dp_aux
drm/bridge: parade-ps8640: Provide wait_hpd_asserted() in struct
drm_dp_aux
drivers/gpu/drm/bridge/parade-ps8640.c| 34 +--
drivers/gpu/drm/panel/panel-edp.c | 33 ++-
.../gpu/drm/panel/panel-samsung-atna33xc20.c | 41 +--
include/drm/dp/drm_dp_helper.h| 26
4 files changed, 98 insertions(+), 36 deletions(-)
--
2.36.0.rc0.470.gd361397f0d-goog
Re: [PATCH v2 3/4] drm/panel: atna33xc20: Take advantage of wait_hpd_asserted() in struct drm_dp_aux
Hi,
On Mon, Apr 18, 2022 at 9:58 AM Douglas Anderson wrote:
>
> Let's add support for being able to read the HPD pin even if it's
> hooked directly to the controller. This will let us take away the
> waiting in the AUX transfer functions of the eDP controller drivers.
>
> Signed-off-by: Douglas Anderson
> ---
>
> Changes in v2:
> - Change is_hpd_asserted() to wait_hpd_asserted()
>
> .../gpu/drm/panel/panel-samsung-atna33xc20.c | 38 ---
> 1 file changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
> b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
> index 20666b6217e7..7f9af3e9aeb8 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
> @@ -19,6 +19,10 @@
> #include
> #include
>
> +/* T3 VCC to HPD high is max 200 ms */
> +#define HPD_MAX_MS 200
> +#define HPD_MAX_US (HPD_MAX_MS * 1000)
> +
> struct atana33xc20_panel {
> struct drm_panel base;
> bool prepared;
> @@ -30,6 +34,7 @@ struct atana33xc20_panel {
>
> struct regulator *supply;
> struct gpio_desc *el_on3_gpio;
> + struct drm_dp_aux *aux;
>
> struct edid *edid;
>
> @@ -90,20 +95,25 @@ static int atana33xc20_resume(struct device *dev)
> return ret;
> p->powered_on_time = ktime_get();
>
> - /*
> -* Handle HPD. Note: if HPD is hooked up to a dedicated pin on the
> -* eDP controller then "no_hpd" will be false _and_ "hpd_gpio" will be
> -* NULL. It's up to the controller driver to wait for HPD after
> -* preparing the panel in that case.
> -*/
> if (p->no_hpd) {
> - /* T3 VCC to HPD high is max 200 ms */
> - msleep(200);
> - } else if (p->hpd_gpio) {
> - ret = readx_poll_timeout(gpiod_get_value_cansleep,
> p->hpd_gpio,
> -hpd_asserted, hpd_asserted,
> -1000, 20);
> - if (!hpd_asserted)
> + msleep(HPD_MAX_MS);
> + } else {
> + if (p->hpd_gpio)
> + ret = readx_poll_timeout(gpiod_get_value_cansleep,
> +p->hpd_gpio, hpd_asserted,
> +hpd_asserted, 1000,
> HPD_MAX_US);
> + else if (p->aux->wait_hpd_asserted)
> + ret = p->aux->wait_hpd_asserted(p->aux, HPD_MAX_US);
> +
> + /*
> +* Note that it's possible that no_hpd is false, hpd_gpio is
> +* NULL, and wait_hpd_asserted is NULL. This is because
> +* wait_hpd_asserted() is optional even if HPD is hooked up to
> +* a dedicated pin on the eDP controller. In this case we just
> +* assume that the controller driver will wait for HPD at the
> +* right times.
> +*/
> + if (!hpd_asserted && (p->hpd_gpio ||
> p->aux->wait_hpd_asserted))
> dev_warn(dev, "Timeout waiting for HPD\n");
Ugh, right after I sent this out I found a bug! :( It should be
checking for "ret", not "hpd_asserted" in the above "if" test. I'll
spin out a quick v3 right away!
-Doug
[PATCH v2 4/4] drm/bridge: parade-ps8640: Provide wait_hpd_asserted() in struct drm_dp_aux
This implements the callback added by the patch ("drm/dp: Add
wait_hpd_asserted() callback to struct drm_dp_aux").
With this change and all the two "DP AUX Endpoint" drivers changed to
use wait_hpd_asserted(), we no longer need to have an long delay in
the AUX transfer function. It's up to the panel code to make sure that
the panel is powered now. If someone tried to call the aux transfer
function without making sure the panel is powered we'll just get a
normal transfer failure.
We'll still keep the wait for HPD in the pre_enable() function. Though
it's probably not actually needed there, this driver is used in the
old mode (pre-DP AUX Endpoints) and it may be important for those
cases. If nothing else, it shouldn't cause any big problems.
Signed-off-by: Douglas Anderson
---
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
drivers/gpu/drm/bridge/parade-ps8640.c | 34 --
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c
b/drivers/gpu/drm/bridge/parade-ps8640.c
index 9766cbbd62ad..2f19a8c89880 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -168,23 +168,30 @@ static bool ps8640_of_panel_on_aux_bus(struct device *dev)
return true;
}
-static int ps8640_ensure_hpd(struct ps8640 *ps_bridge)
+static int _ps8640_wait_hpd_asserted(struct ps8640 *ps_bridge, unsigned long
wait_us)
{
struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL];
- struct device *dev = _bridge->page[PAGE2_TOP_CNTL]->dev;
int status;
- int ret;
/*
* Apparently something about the firmware in the chip signals that
* HPD goes high by reporting GPIO9 as high (even though HPD isn't
* actually connected to GPIO9).
*/
- ret = regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
- status & PS_GPIO9, 20 * 1000, 200 *
1000);
+ return regmap_read_poll_timeout(map, PAGE2_GPIO_H, status,
+ status & PS_GPIO9, wait_us / 10,
wait_us);
+}
- if (ret < 0)
- dev_warn(dev, "HPD didn't go high: %d\n", ret);
+static int ps8640_wait_hpd_asserted(struct drm_dp_aux *aux, unsigned long
wait_us)
+{
+ struct ps8640 *ps_bridge = aux_to_ps8640(aux);
+ struct device *dev = _bridge->page[PAGE0_DP_CNTL]->dev;
+ int ret;
+
+ pm_runtime_get_sync(dev);
+ ret = _ps8640_wait_hpd_asserted(ps_bridge, wait_us);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
return ret;
}
@@ -323,9 +330,7 @@ static ssize_t ps8640_aux_transfer(struct drm_dp_aux *aux,
int ret;
pm_runtime_get_sync(dev);
- ret = ps8640_ensure_hpd(ps_bridge);
- if (!ret)
- ret = ps8640_aux_transfer_msg(aux, msg);
+ ret = ps8640_aux_transfer_msg(aux, msg);
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
@@ -369,8 +374,8 @@ static int __maybe_unused ps8640_resume(struct device *dev)
* Mystery 200 ms delay for the "MCU to be ready". It's unclear if
* this is truly necessary since the MCU will already signal that
* things are "good to go" by signaling HPD on "gpio 9". See
-* ps8640_ensure_hpd(). For now we'll keep this mystery delay just in
-* case.
+* _ps8640_wait_hpd_asserted(). For now we'll keep this mystery delay
+* just in case.
*/
msleep(200);
@@ -406,7 +411,9 @@ static void ps8640_pre_enable(struct drm_bridge *bridge)
int ret;
pm_runtime_get_sync(dev);
- ps8640_ensure_hpd(ps_bridge);
+ ret = _ps8640_wait_hpd_asserted(ps_bridge, 200 * 1000);
+ if (ret < 0)
+ dev_warn(dev, "HPD didn't go high: %d\n", ret);
/*
* The Manufacturer Command Set (MCS) is a device dependent interface
@@ -652,6 +659,7 @@ static int ps8640_probe(struct i2c_client *client)
ps_bridge->aux.name = "parade-ps8640-aux";
ps_bridge->aux.dev = dev;
ps_bridge->aux.transfer = ps8640_aux_transfer;
+ ps_bridge->aux.wait_hpd_asserted = ps8640_wait_hpd_asserted;
drm_dp_aux_init(_bridge->aux);
pm_runtime_enable(dev);
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH v2 3/4] drm/panel: atna33xc20: Take advantage of wait_hpd_asserted() in struct drm_dp_aux
Let's add support for being able to read the HPD pin even if it's
hooked directly to the controller. This will let us take away the
waiting in the AUX transfer functions of the eDP controller drivers.
Signed-off-by: Douglas Anderson
---
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
.../gpu/drm/panel/panel-samsung-atna33xc20.c | 38 ---
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
index 20666b6217e7..7f9af3e9aeb8 100644
--- a/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
+++ b/drivers/gpu/drm/panel/panel-samsung-atna33xc20.c
@@ -19,6 +19,10 @@
#include
#include
+/* T3 VCC to HPD high is max 200 ms */
+#define HPD_MAX_MS 200
+#define HPD_MAX_US (HPD_MAX_MS * 1000)
+
struct atana33xc20_panel {
struct drm_panel base;
bool prepared;
@@ -30,6 +34,7 @@ struct atana33xc20_panel {
struct regulator *supply;
struct gpio_desc *el_on3_gpio;
+ struct drm_dp_aux *aux;
struct edid *edid;
@@ -90,20 +95,25 @@ static int atana33xc20_resume(struct device *dev)
return ret;
p->powered_on_time = ktime_get();
- /*
-* Handle HPD. Note: if HPD is hooked up to a dedicated pin on the
-* eDP controller then "no_hpd" will be false _and_ "hpd_gpio" will be
-* NULL. It's up to the controller driver to wait for HPD after
-* preparing the panel in that case.
-*/
if (p->no_hpd) {
- /* T3 VCC to HPD high is max 200 ms */
- msleep(200);
- } else if (p->hpd_gpio) {
- ret = readx_poll_timeout(gpiod_get_value_cansleep, p->hpd_gpio,
-hpd_asserted, hpd_asserted,
-1000, 20);
- if (!hpd_asserted)
+ msleep(HPD_MAX_MS);
+ } else {
+ if (p->hpd_gpio)
+ ret = readx_poll_timeout(gpiod_get_value_cansleep,
+p->hpd_gpio, hpd_asserted,
+hpd_asserted, 1000,
HPD_MAX_US);
+ else if (p->aux->wait_hpd_asserted)
+ ret = p->aux->wait_hpd_asserted(p->aux, HPD_MAX_US);
+
+ /*
+* Note that it's possible that no_hpd is false, hpd_gpio is
+* NULL, and wait_hpd_asserted is NULL. This is because
+* wait_hpd_asserted() is optional even if HPD is hooked up to
+* a dedicated pin on the eDP controller. In this case we just
+* assume that the controller driver will wait for HPD at the
+* right times.
+*/
+ if (!hpd_asserted && (p->hpd_gpio || p->aux->wait_hpd_asserted))
dev_warn(dev, "Timeout waiting for HPD\n");
}
@@ -263,6 +273,8 @@ static int atana33xc20_probe(struct dp_aux_ep_device
*aux_ep)
return -ENOMEM;
dev_set_drvdata(dev, panel);
+ panel->aux = aux_ep->aux;
+
panel->supply = devm_regulator_get(dev, "power");
if (IS_ERR(panel->supply))
return dev_err_probe(dev, PTR_ERR(panel->supply),
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH v2 2/4] drm/panel-edp: Take advantage of wait_hpd_asserted() in struct drm_dp_aux
Let's add support for being able to read the HPD pin even if it's
hooked directly to the controller. This will allow us to get more
accurate delays also lets us take away the waiting in the AUX transfer
functions of the eDP controller drivers.
Signed-off-by: Douglas Anderson
---
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
drivers/gpu/drm/panel/panel-edp.c | 33 +--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 1732b4f56e38..086e0bf52fb9 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -417,6 +417,11 @@ static int panel_edp_get_hpd_gpio(struct device *dev,
struct panel_edp *p)
return 0;
}
+static bool panel_edp_can_read_hpd(struct panel_edp *p)
+{
+ return !p->no_hpd && (p->hpd_gpio || (p->aux &&
p->aux->wait_hpd_asserted));
+}
+
static int panel_edp_prepare_once(struct panel_edp *p)
{
struct device *dev = p->base.dev;
@@ -441,17 +446,21 @@ static int panel_edp_prepare_once(struct panel_edp *p)
if (delay)
msleep(delay);
- if (p->hpd_gpio) {
+ if (panel_edp_can_read_hpd(p)) {
if (p->desc->delay.hpd_absent)
hpd_wait_us = p->desc->delay.hpd_absent * 1000UL;
else
hpd_wait_us = 200;
- err = readx_poll_timeout(gpiod_get_value_cansleep, p->hpd_gpio,
-hpd_asserted, hpd_asserted,
-1000, hpd_wait_us);
- if (hpd_asserted < 0)
- err = hpd_asserted;
+ if (p->hpd_gpio) {
+ err = readx_poll_timeout(gpiod_get_value_cansleep,
+p->hpd_gpio, hpd_asserted,
+hpd_asserted, 1000,
hpd_wait_us);
+ if (hpd_asserted < 0)
+ err = hpd_asserted;
+ } else {
+ err = p->aux->wait_hpd_asserted(p->aux, hpd_wait_us);
+ }
if (err) {
if (err != -ETIMEDOUT)
@@ -532,18 +541,22 @@ static int panel_edp_enable(struct drm_panel *panel)
/*
* If there is a "prepare_to_enable" delay then that's supposed to be
* the delay from HPD going high until we can turn the backlight on.
-* However, we can only count this if HPD is handled by the panel
-* driver, not if it goes to a dedicated pin on the controller.
+* However, we can only count this if HPD is readable by the panel
+* driver.
+*
* If we aren't handling the HPD pin ourselves then the best we
* can do is assume that HPD went high immediately before we were
-* called (and link training took zero time).
+* called (and link training took zero time). Note that "no-hpd"
+* actually counts as handling HPD ourselves since we're doing the
+* worst case delay (in prepare) ourselves.
*
* NOTE: if we ever end up in this "if" statement then we're
* guaranteed that the panel_edp_wait() call below will do no delay.
* It already handles that case, though, so we don't need any special
* code for it.
*/
- if (p->desc->delay.prepare_to_enable && !p->hpd_gpio && !p->no_hpd)
+ if (p->desc->delay.prepare_to_enable &&
+ !panel_edp_can_read_hpd(p) && !p->no_hpd)
delay = max(delay, p->desc->delay.prepare_to_enable);
if (delay)
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH v2 1/4] drm/dp: Add wait_hpd_asserted() callback to struct drm_dp_aux
Sometimes it's useful for users of the DP AUX bus (like panels) to be
able to poll HPD. Let's add a callback that allows DP AUX busses
drivers to provide this.
Suggested-by: Dmitry Baryshkov
Signed-off-by: Douglas Anderson
---
Left Dmitry's Reviewed-by tag off since patch changed enough.
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
include/drm/dp/drm_dp_helper.h | 26 ++
1 file changed, 26 insertions(+)
diff --git a/include/drm/dp/drm_dp_helper.h b/include/drm/dp/drm_dp_helper.h
index 53d1e722f4de..0940c415db8c 100644
--- a/include/drm/dp/drm_dp_helper.h
+++ b/include/drm/dp/drm_dp_helper.h
@@ -2035,6 +2035,32 @@ struct drm_dp_aux {
ssize_t (*transfer)(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg);
+ /**
+* @wait_hpd_asserted: wait for HPD to be asserted
+*
+* This is mainly useful for eDP panels drivers to wait for an eDP
+* panel to finish powering on. This is an optional function.
+*
+* This function will efficiently wait for up to `wait_us` microseconds
+* for HPD to be asserted and might sleep.
+*
+* This function returns 0 if HPD was asserted or -ETIMEDOUT if time
+* expired and HPD wasn't asserted. This function should not print
+* timeout errors to the log.
+*
+* The semantics of this function are designed to match the
+* readx_poll_timeout() function. That means a `wait_us` of 0 means
+* to wait forever. If you want to do a quick poll you could pass 1
+* for `wait_us`.
+*
+* NOTE: this function specifically reports the state of the HPD pin
+* that's associated with the DP AUX channel. This is different from
+* the HPD concept in much of the rest of DRM which is more about
+* physical presence of a display. For eDP, for instance, a display is
+* assumed always present even if the HPD pin is deasserted.
+*/
+ int (*wait_hpd_asserted)(struct drm_dp_aux *aux, unsigned long wait_us);
+
/**
* @i2c_nack_count: Counts I2C NACKs, used for DP validation.
*/
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH v2 0/4] drm/dp: Introduce wait_hpd_asserted() for the DP AUX bus
This is the 2nd four patches from my RFC series ("drm/dp: Improvements
for DP AUX channel") [1]. I've broken the series in two so we can make
progress on the two halves separately.
v2 of this series changes to add wait_hpd_asserted() instead of
is_hpd_asserted(). This allows us to move the extra delay needed for
ps8640 into the ps8640 driver itself.
The idea for this series came up during the review process of
Sankeerth's series trying to add eDP for Qualcomm SoCs [2].
This _doesn't_ attempt to fix the Analogix driver. If this works out,
ideally someone can post a patch up to do that.
[1] https://lore.kernel.org/r/20220409023628.2104952-1-diand...@chromium.org/
[2]
https://lore.kernel.org/r/1648656179-10347-2-git-send-email-quic_sbill...@quicinc.com/
Changes in v2:
- Change is_hpd_asserted() to wait_hpd_asserted()
Douglas Anderson (4):
drm/dp: Add wait_hpd_asserted() callback to struct drm_dp_aux
drm/panel-edp: Take advantage of wait_hpd_asserted() in struct
drm_dp_aux
drm/panel: atna33xc20: Take advantage of wait_hpd_asserted() in struct
drm_dp_aux
drm/bridge: parade-ps8640: Provide wait_hpd_asserted() in struct
drm_dp_aux
drivers/gpu/drm/bridge/parade-ps8640.c| 34 ++---
drivers/gpu/drm/panel/panel-edp.c | 33 +++-
.../gpu/drm/panel/panel-samsung-atna33xc20.c | 38 ---
include/drm/dp/drm_dp_helper.h| 26 +
4 files changed, 95 insertions(+), 36 deletions(-)
--
2.36.0.rc0.470.gd361397f0d-goog
Re: [PATCH v9] drm/msm/dp: stop event kernel thread when DP unbind
On 16/04/2022 02:47, Kuogee Hsieh wrote:
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(>event_q) to probe()
-- move spin_lock_init(>event_lock) to probe()
Changes in v4:
-- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
Changes in v5:
-- cancel relocation of both dp_display_bind() and dp_display_unbind()
Changes in v6:
-- move empty event q to dp_event_thread_start()
Changes in v7:
-- call ktheread_stop() directly instead of dp_hpd_event_thread_stop() function
Changes in v8:
-- return error immediately if audio registration failed.
Changes in v9:
-- return error immediately if event thread create failed.
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
Reviewed-by: Stephen Boyd
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dp/dp_display.c | 41 +
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..5b289b9 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,8 @@ void dp_display_signal_audio_complete(struct msm_dp *dp_display)
complete_all(>audio_comp);
}
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -266,9 +269,18 @@ static int dp_display_bind(struct device *dev, struct
device *master,
}
rc = dp_register_audio_driver(dev, dp->audio);
- if (rc)
+ if (rc) {
DRM_ERROR("Audio registration Dp failed\n");
+ goto end;
+ }
+ rc = dp_hpd_event_thread_start(dp);
+ if (rc) {
+ DRM_ERROR("Event thread create failed\n");
+ goto end;
+ }
+
+ return 0;
end:
return rc;
}
@@ -280,6 +292,11 @@ 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);
+
+ kthread_stop(dp->ev_tsk);
+
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1071,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,12 +1149,19 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
{
- init_waitqueue_head(_priv->event_q);
- spin_lock_init(_priv->event_lock);
+ /* set event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 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");
+ return PTR_ERR(dp_priv->ev_tsk);
+ }
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
+ return 0;
}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
@@ -1266,7 +1290,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(>event_mutex);
+ init_waitqueue_head(>event_q);
+ spin_lock_init(>event_lock);
/* Store DP audio handle inside DP display */
dp->dp_display.dp_audio = dp->audio;
@@ -1441,8 +1468,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);
}
--
With best wishes
Dmitry
Re: [PATCH 1/9] vfio: Make vfio_(un)register_notifier accept a vfio_device
On Mon, Apr 18, 2022 at 11:28:30AM -0400, Tony Krowiak wrote:
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > index a4555014bd1e72..8a5c46aa2bef61 100644
> > +++ b/drivers/vfio/vfio.c
> > @@ -2484,19 +2484,15 @@ static int vfio_unregister_group_notifier(struct
> > vfio_group *group,
> > return ret;
> > }
> > -int vfio_register_notifier(struct device *dev, enum vfio_notify_type type,
> > +int vfio_register_notifier(struct vfio_device *dev, enum vfio_notify_type
> > type,
> >unsigned long *events, struct notifier_block *nb)
> > {
> > - struct vfio_group *group;
> > + struct vfio_group *group = dev->group;
>
> Is there a guarantee that dev != NULL? The original code below checks
> the value of dev, so why is that check eliminated here?
Yes, no kernel driver calls this with null dev. The original code
should have been a WARN_ON as it is just protecting against a buggy
driver. In this case if the driver is buggy we simply generate a
backtrace through a null deref panic.
Jason
[PATCH] drm/nouveau/gr/gf100-: change gf108_gr_fwif from global to static
Smatch reports this issue
gf108.c:147:1: warning: symbol 'gf108_gr_fwif'
was not declared. Should it be static?
gf108_gr_fwif is only used in gf108.c. Single
file variables should not be global so change
gf108_gr_fwif's storage-class specifier to static.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c
b/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c
index 030640bb3dca..ab3760e804b8 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/gr/gf108.c
@@ -143,7 +143,7 @@ gf108_gr = {
}
};
-const struct gf100_gr_fwif
+static const struct gf100_gr_fwif
gf108_gr_fwif[] = {
{ -1, gf100_gr_load, _gr },
{ -1, gf100_gr_nofw, _gr },
--
2.27.0
[PATCH v2 2/2] drm: bridge: ldb: Implement simple NXP i.MX8M LDB bridge
The i.MX8MP contains two syscon registers which are responsible
for configuring the on-SoC DPI-to-LVDS serializer. Implement a
simple bridge driver for this serializer.
Signed-off-by: Marek Vasut
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Maxime Ripard
Cc: Peng Fan
Cc: Robby Cai
Cc: Robert Foss
Cc: Sam Ravnborg
Cc: Thomas Zimmermann
To: dri-devel@lists.freedesktop.org
--
V2: - Rename syscon to fsl,syscon
---
drivers/gpu/drm/bridge/Kconfig | 8 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/nxp-ldb.c | 343 +++
3 files changed, 352 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/nxp-ldb.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 20f9bc7f4be54..7fe7088a2bef5 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -185,6 +185,14 @@ config DRM_NWL_MIPI_DSI
This enables the Northwest Logic MIPI DSI Host controller as
for example found on NXP's i.MX8 Processors.
+config DRM_NXP_LDB
+ tristate "NXP i.MX8M LDB bridge"
+ depends on OF
+ select DRM_KMS_HELPER
+ select DRM_PANEL_BRIDGE
+ help
+ Support for i.MX8M DPI-to-LVDS on-SoC encoder.
+
config DRM_NXP_PTN3460
tristate "NXP PTN3460 DP/LVDS bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index bdffad2a7ed3a..f800b2331d9e0 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o
obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o
obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) +=
megachips-stdp-ge-b850v3-fw.o
+obj-$(CONFIG_DRM_NXP_LDB) += nxp-ldb.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
diff --git a/drivers/gpu/drm/bridge/nxp-ldb.c b/drivers/gpu/drm/bridge/nxp-ldb.c
new file mode 100644
index 0..7b8de235876ea
--- /dev/null
+++ b/drivers/gpu/drm/bridge/nxp-ldb.c
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2022 Marek Vasut
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#define LDB_CTRL 0x5c
+#define LDB_CTRL_CH0_ENABLEBIT(0)
+#define LDB_CTRL_CH0_DI_SELECT BIT(1)
+#define LDB_CTRL_CH1_ENABLEBIT(2)
+#define LDB_CTRL_CH1_DI_SELECT BIT(3)
+#define LDB_CTRL_SPLIT_MODEBIT(4)
+#define LDB_CTRL_CH0_DATA_WIDTHBIT(5)
+#define LDB_CTRL_CH0_BIT_MAPPING BIT(6)
+#define LDB_CTRL_CH1_DATA_WIDTHBIT(7)
+#define LDB_CTRL_CH1_BIT_MAPPING BIT(8)
+#define LDB_CTRL_DI0_VSYNC_POLARITYBIT(9)
+#define LDB_CTRL_DI1_VSYNC_POLARITYBIT(10)
+#define LDB_CTRL_REG_CH0_FIFO_RESETBIT(11)
+#define LDB_CTRL_REG_CH1_FIFO_RESETBIT(12)
+#define LDB_CTRL_ASYNC_FIFO_ENABLE BIT(24)
+#define LDB_CTRL_ASYNC_FIFO_THRESHOLD_MASK GENMASK(27, 25)
+
+#define LVDS_CTRL 0x128
+#define LVDS_CTRL_CH0_EN BIT(0)
+#define LVDS_CTRL_CH1_EN BIT(1)
+#define LVDS_CTRL_VBG_EN BIT(2)
+#define LVDS_CTRL_HS_ENBIT(3)
+#define LVDS_CTRL_PRE_EMPH_EN BIT(4)
+#define LVDS_CTRL_PRE_EMPH_ADJ(n) (((n) & 0x7) << 5)
+#define LVDS_CTRL_PRE_EMPH_ADJ_MASKGENMASK(7, 5)
+#define LVDS_CTRL_CM_ADJ(n)(((n) & 0x7) << 8)
+#define LVDS_CTRL_CM_ADJ_MASK GENMASK(10, 8)
+#define LVDS_CTRL_CC_ADJ(n)(((n) & 0x7) << 11)
+#define LVDS_CTRL_CC_ADJ_MASK GENMASK(13, 11)
+#define LVDS_CTRL_SLEW_ADJ(n) (((n) & 0x7) << 14)
+#define LVDS_CTRL_SLEW_ADJ_MASKGENMASK(16, 14)
+#define LVDS_CTRL_VBG_ADJ(n) (((n) & 0x7) << 17)
+#define LVDS_CTRL_VBG_ADJ_MASK GENMASK(19, 17)
+
+struct nxp_ldb {
+ struct device *dev;
+ struct drm_bridge bridge;
+ struct drm_bridge *panel_bridge;
+ struct clk *clk;
+ struct regmap *regmap;
+ bool lvds_dual_link;
+};
+
+static inline struct nxp_ldb *to_nxp_ldb(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct nxp_ldb, bridge);
+}
+
+static int nxp_ldb_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct nxp_ldb *nxp_ldb = to_nxp_ldb(bridge);
+
+ return drm_bridge_attach(bridge->encoder, nxp_ldb->panel_bridge,
+bridge, flags);
+}
+
[PATCH v2 1/2] dt-bindings: display: bridge: ldb: Implement simple NXP i.MX8M LDB bridge
The i.MX8MP contains two syscon registers which are responsible
for configuring the on-SoC DPI-to-LVDS serializer. Add DT binding
which represents this serializer as a bridge.
Signed-off-by: Marek Vasut
Cc: Laurent Pinchart
Cc: Lucas Stach
Cc: Maxime Ripard
Cc: Peng Fan
Cc: Rob Herring
Cc: Robby Cai
Cc: Robert Foss
Cc: Sam Ravnborg
Cc: Thomas Zimmermann
Cc: devicet...@vger.kernel.org
To: dri-devel@lists.freedesktop.org
---
V2: - Consistently use fsl,imx8mp-ldb as compatible
- Drop items: from compatible:
- Replace minItems with maxItems in clocks:
- Drop quotes from clock-names const: ldb
- Rename syscon to fsl,syscon
- Use generic name of ldb-lvds in example
---
.../bindings/display/bridge/nxp,ldb.yaml | 96 +++
1 file changed, 96 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
new file mode 100644
index 0..f3182566eb316
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/nxp,ldb.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/nxp,ldb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8M DPI to LVDS bridge chip
+
+maintainers:
+ - Marek Vasut
+
+description: |
+ The i.MX8MP contains two syscon registers which are responsible
+ for configuring the on-SoC DPI-to-LVDS serializer. This describes
+ those registers as bridge within the DT.
+
+properties:
+ compatible:
+const: fsl,imx8mp-ldb
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+const: ldb
+
+ fsl,syscon:
+$ref: /schemas/types.yaml#/definitions/phandle
+description: A phandle to media block controller.
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/properties/port
+description: Video port for DPI input.
+
+ port@1:
+$ref: /schemas/graph.yaml#/properties/port
+description: Video port for LVDS Channel-A output (panel or bridge).
+
+ port@2:
+$ref: /schemas/graph.yaml#/properties/port
+description: Video port for LVDS Channel-B output (panel or bridge).
+
+required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - clocks
+ - fsl,syscon
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+bridge {
+compatible = "fsl,imx8mp-ldb";
+clocks = < IMX8MP_CLK_MEDIA_LDB>;
+clock-names = "ldb";
+fsl,syscon = <_blk_ctrl>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+
+ldb_from_lcdif2: endpoint {
+remote-endpoint = <_to_ldb>;
+};
+};
+
+port@1 {
+reg = <1>;
+
+ldb_lvds_ch0: endpoint {
+remote-endpoint = <_to_lvdsx4panel>;
+};
+};
+
+port@2 {
+reg = <2>;
+
+ldb_lvds_ch1: endpoint {
+};
+};
+};
+};
--
2.35.1
[PATCH] drm/nouveau: change base917c_format from global to static
Smatch reports this issue
base917c.c:26:1: warning: symbol 'base917c_format'
was not declared. Should it be static?
base917c_format is only used in base917.c. Single
file variables should not be global so change
base917c_format's storage-class specifier to static.
Signed-off-by: Tom Rix
---
drivers/gpu/drm/nouveau/dispnv50/base917c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/base917c.c
b/drivers/gpu/drm/nouveau/dispnv50/base917c.c
index a1baed4fe0e9..ca260509a4f1 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/base917c.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/base917c.c
@@ -22,7 +22,7 @@
#include "base.h"
#include "atom.h"
-const u32
+static const u32
base917c_format[] = {
DRM_FORMAT_C8,
DRM_FORMAT_XRGB,
--
2.27.0
[PATCH 4.9 133/218] 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 = >algo;
--
2.34.1
[PATCH 4.14 165/284] 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 = >algo;
--
2.34.1
Re: [PATCH] drm/panel: newvision-nv3052c: Fix build
On Mon, 18 Apr 2022 at 17:07, Sam Ravnborg wrote:
>
> On Sat, Apr 09, 2022 at 11:00:16AM +0100, Paul Cercueil wrote:
> > The driver was compile-tested then rebased on drm-misc-next, and not
> > compile-tested after the rebase; unfortunately the driver didn't compile
> > anymore when it hit drm-misc-next.
> >
> > Fixes: 49956b505c53 ("drm/panel: Add panel driver for NewVision NV3052C
> > based LCDs")
> > Cc: Christophe Branchereau
> > Cc: kbuild-all
> > Cc: Stephen Rothwell
> > Reported-by: kernel test robot
> > Signed-off-by: Paul Cercueil
> Acked-by: Sam Ravnborg
> > ---
Backmerge drm-next. I fixed this up when I merged this driver in the
merge commit.
Dave.
[Bug 212499] nouveau locking issue - WARNING: possible circular locking dependency detected
https://bugzilla.kernel.org/show_bug.cgi?id=212499 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |OBSOLETE --- Comment #2 from Erhard F. (erhar...@mailbox.org) --- Sold the hardware, no longer able to test. -- You may reply to this email to add a comment. You are receiving this mail because: You are on the CC list for the bug. You are watching the assignee of the bug.
[Bug 212635] nouveau 0000:04:00.0: fifo: fault 00 [READ] at 0000000000380000 engine 00 [GR] client 14 [HUB/SCC] reason 02 [PTE] on channel 5 [007fabf000 X[570]]
https://bugzilla.kernel.org/show_bug.cgi?id=212635 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |OBSOLETE --- Comment #2 from Erhard F. (erhar...@mailbox.org) --- Sold the hardware, no longer able to test. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] drm/panel: newvision-nv3052c: Fix build
On Sat, Apr 09, 2022 at 11:00:16AM +0100, Paul Cercueil wrote:
> The driver was compile-tested then rebased on drm-misc-next, and not
> compile-tested after the rebase; unfortunately the driver didn't compile
> anymore when it hit drm-misc-next.
>
> Fixes: 49956b505c53 ("drm/panel: Add panel driver for NewVision NV3052C based
> LCDs")
> Cc: Christophe Branchereau
> Cc: kbuild-all
> Cc: Stephen Rothwell
> Reported-by: kernel test robot
> Signed-off-by: Paul Cercueil
Acked-by: Sam Ravnborg
> ---
> drivers/gpu/drm/panel/panel-newvision-nv3052c.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> index 127bcfdb59df..cf078f0d3cd3 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
> @@ -416,15 +416,13 @@ static int nv3052c_probe(struct spi_device *spi)
> return 0;
> }
>
> -static int nv3052c_remove(struct spi_device *spi)
> +static void nv3052c_remove(struct spi_device *spi)
> {
> struct nv3052c *priv = spi_get_drvdata(spi);
>
> drm_panel_remove(>panel);
> drm_panel_disable(>panel);
> drm_panel_unprepare(>panel);
> -
> - return 0;
> }
>
> static const struct drm_display_mode ltk035c5444t_modes[] = {
> --
> 2.35.1
Re: [PATCH 3/4] dt-bindings: drm/bridge: anx7625: Change bus-type to 7 (DPI)
Hi,
On Thu, Apr 14, 2022 at 10:27 AM Xin Ji wrote:
>
> On Wed, Apr 13, 2022 at 04:28:51PM +0200, Robert Foss wrote:
> > On Sat, 9 Apr 2022 at 06:47, Xin Ji wrote:
> > >
> > > On Mon, Apr 04, 2022 at 12:52:14PM -0500, Rob Herring wrote:
> > > > On Mon, Mar 28, 2022 at 08:09:54PM +0800, Xin Ji wrote:
> > > > > Change bus-type define for DPI.
> > > > >
> > > > > Fixes: a43661e7e819 ("dt-bindings:drm/bridge:anx7625:add vendor
> > > > > define")
> > > > >
> > > > > Signed-off-by: Xin Ji
> > > > > ---
> > > > > .../devicetree/bindings/display/bridge/analogix,anx7625.yaml | 4
> > > > > ++--
> > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > >
> > > > > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > > index 0d38d6fe3983..4590186c4a0b 100644
> > > > > ---
> > > > > a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > > +++
> > > > > b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > > @@ -106,7 +106,7 @@ properties:
> > > > >remote-endpoint: true
> > > > >
> > > > >bus-type:
> > > > > -enum: [1, 5]
> > > > > +enum: [7]
> > > >
> > > > Changing is an ABI break, but didn't we revert adding this?
> > > Hi Rob, sorry, what do you mean about ABI break? Do I need remove this
> > > patch in this serial? Or do I need revert patch
> > > https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F462331%2Fdata=04%7C01%7Cxji%40analogixsemi.com%7C10f5b0213f434592936008da1d59f566%7Cb099b0b4f26c4cf59a0fd5be9acab205%7C0%7C0%7C637854569490105386%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=vK0Vmb9S425kHZc1WurfnNhnoXDMbUGkkdY1PVnfS9g%3Dreserved=0,
> > > I don't know how to do
> > > it.
> > >
> >
> > I contributed to the confusion about this, let's see if we can clear it up.
> >
> > An issue was found related to which enum values were used to represent
> > what late in the last release cycle. As a result a revert[1] patch for
> > everything touching bus-type in anx7625 was merged.
> >
> > This patch, does not apply to drm-misc-next due to the revert, and if
> > Xin Ji rebases his work on the drm-misc-next there should be no
> > ABI-change as this patch when fixed up will introduce bus-type to the
> > nax7625 ABI.
> >
> > Xin: Does this make sense to you?
> Hi Robert Foss, yes, my work is based on drm-misc-next, all I can do,
> just make a fix up patch(this patch) to change the bus-type define.
The revert was applied to the soc tree and merged into Linus's tree
in v5.17-rc8. It was not present in drm-misc-next until April 5 with
commit 9cbbd694a58b ("Merge drm/drm-next into drm-misc-next").
So please fetch the latest drm-misc-next, rebase your patches on top, and
resend.
Thanks
ChenYu
Re: [PATCH v6 2/2] arm64: dts: mt8192: Add node for the Mali GPU
On Thu, Apr 14, 2022 at 10:53 AM Nick Fan wrote: > > Add a basic GPU node for mt8192. > > Signed-off-by: Nick Fan Reviewed-by: Fei Shao
