[PATCH v6 00/10] drm: bridge: Add Samsung MIPI DSIM bridge
This series supports common bridge support for Samsung MIPI DSIM which is used in Exynos and i.MX8MM SoC's. The final bridge supports both the Exynos and i.MX8MM DSI devices. Changes for v6: * handle previous bridge for exynos dsi while attaching bridge Changes for v5: * bridge changes to support multi-arch * updated and clear commit messages * add hw_type via plat data * removed unneeded quirk * rebased on linux-next Changes for v4: * include Inki Dae in MAINTAINERS * remove dsi_driver probe in exynos_drm_drv to support multi-arch build * update init handling to ensure host init done on first cmd transfer Changes for v3: * fix the mult-arch build * fix dsi host init * updated commit messages Changes for v2: * fix bridge handling * fix dsi host init * correct the commit messages Patch 0001: Samsung DSIM bridge Patch 0002: PHY optional Patch 0003: OF-graph or Child node lookup Patch 0004: DSI host initialization Patch 0005: atomic check Patch 0006: PMS_P offset via plat data Patch 0007: atomic_get_input_bus_fmts Patch 0008: input_bus_flags Patch 0009: document fsl,imx8mm-mipi-dsim Patch 0010: add i.MX8MM DSIM support Tested in Engicam i.Core MX8M Mini SoM. Repo: https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v6 Any inputs? Jagan. Jagan Teki (10): drm: bridge: Add Samsung DSIM bridge driver drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices drm: bridge: samsung-dsim: Mark PHY as optional drm: bridge: samsung-dsim: Handle proper DSI host initialization drm: bridge: samsung-dsim: Add atomic_check drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts drm: bridge: samsung-dsim: Add input_bus_flags dt-bindings: display: exynos: dsim: Add NXP i.MX8MM support drm: bridge: samsung-dsim: Add i.MX8MM support .../bindings/display/exynos/exynos_dsim.txt |1 + MAINTAINERS |9 + drivers/gpu/drm/bridge/Kconfig| 12 + drivers/gpu/drm/bridge/Makefile |1 + drivers/gpu/drm/bridge/samsung-dsim.c | 1856 + drivers/gpu/drm/exynos/Kconfig|1 + drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1769 ++-- include/drm/bridge/samsung-dsim.h | 115 + 8 files changed, 2111 insertions(+), 1653 deletions(-) create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c create mode 100644 include/drm/bridge/samsung-dsim.h -- 2.25.1
[PATCH v6 01/10] drm: bridge: Add Samsung DSIM bridge driver
Samsung MIPI DSIM controller is common DSI IP that can be used in various
SoCs like Exynos, i.MX8M Mini/Nano.
In order to access this DSI controller between various platform SoCs,
the ideal way to incorporate this in the drm stack is via the drm bridge
driver.
This patch is trying to differentiate platform-specific and bridge driver
code by maintaining exynos platform glue code in exynos_drm_dsi.c driver
and common bridge driver code in samsung-dsim.c providing that the new
platform-specific glue should be supported in the bridge driver, unlike
exynos platform drm drivers.
- Add samsung_dsim_plat_data for keeping platform-specific attributes like
host_ops, irq_ops, and hw_type.
- Initialize the plat_data hooks for exynos platform in exynos_drm_dsi.c.
- samsung_dsim_probe is the common probe call across exynos_drm_dsi.c and
samsung-dsim.c.
- plat_data hooks like host_ops and irq_ops are invoked during the
respective bridge call chains.
v6:
* handle previous bridge pointer for exynos dsi
v5:
* [mszyprow] reworked driver initialization using the same approach as in
drivers/gpu/drm/{exynos/exynos_dp.c, bridge/analogix/analogix_dp_core.c},
removed weak functions, moved exynos_dsi_driver back to exynos_drm_dsi.c
and restored integration with exynos_drm custom initialization.
* updated the commit message [Jagan]
v4:
* include Inki Dae in MAINTAINERS
* remove dsi_driver probe in exynos_drm_drv to support multi-arch build
v3:
* restore gpio related fixes
* restore proper bridge chain
* rework initialization issue
* fix header includes in proper way
v2:
* fixed exynos dsi driver conversion (Marek Szyprowski)
* updated commit message
* updated MAINTAINERS file
v1:
* don't maintain component_ops in bridge driver
* don't maintain platform glue code in bridge driver
* add platform-specific glue code and make a common bridge
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
MAINTAINERS |9 +
drivers/gpu/drm/bridge/Kconfig | 12 +
drivers/gpu/drm/bridge/Makefile |1 +
drivers/gpu/drm/bridge/samsung-dsim.c | 1703 ++
drivers/gpu/drm/exynos/Kconfig |1 +
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1769 ++-
include/drm/bridge/samsung-dsim.h | 113 ++
7 files changed, 1955 insertions(+), 1653 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.c
create mode 100644 include/drm/bridge/samsung-dsim.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ae989b32ebb..ba7a6721443c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6624,6 +6624,15 @@ T: git git://anongit.freedesktop.org/drm/drm-misc
F: Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml
F: drivers/gpu/drm/panel/panel-samsung-db7430.c
+DRM DRIVER FOR SAMSUNG MIPI DSIM BRIDGE
+M: Jagan Teki
+M: Marek Szyprowski
+M: Inki Dae
S: Maintained
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 57946d80b02d..8e85dac9f53e 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -231,6 +231,18 @@ config DRM_PARADE_PS8640
The PS8640 is a high-performance and low-power
MIPI DSI to eDP converter
+config DRM_SAMSUNG_DSIM
+ tristate "Samsung MIPI DSIM bridge driver"
+ depends on COMMON_CLK
+ depends on OF && HAS_IOMEM
+ select DRM_KMS_HELPER
+ select DRM_MIPI_DSI
+ select DRM_PANEL_BRIDGE
+ help
+ The Samsung MIPI DSIM bridge controller driver.
+ This MIPI DSIM bridge can be found it on Exynos SoCs and
+ NXP's i.MX8M Mini/Nano.
+
config DRM_SIL_SII8620
tristate "Silicon Image SII8620 HDMI/MHL bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 1884803c6860..dae843723991 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) +=
megachips-stdp-ge-b850v
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
+obj-$(CONFIG_DRM_SAMSUNG_DSIM) += samsung-dsim.o
obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
obj-$(CONFIG_DRM_SII902X) += sii902x.o
obj-$(CONFIG_DRM_SII9234) += sii9234.o
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
new file mode 100644
index ..73dcd825c654
--- /dev/null
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -0,0 +1,1703 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Samsung MIPI DSIM bridge driver.
+ *
+ * Copyright (C) 2021 Amarula Solutions(India)
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd
+ * Author: Jagan Teki
+ *
+ * Based on exynos_drm_dsi from
+ * Tomasz Figa
+ */
+
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
[PATCH v6 02/10] drm: bridge: samsung-dsim: Lookup OF-graph or Child node devices
The child devices in MIPI DSI can be binding with OF-graph
and also via child nodes.
The OF-graph interface represents the child devices via
remote and associated endpoint numbers like
dsi {
compatible = "fsl,imx8mm-mipi-dsim";
ports {
port@0 {
reg = <0>;
dsi_in_lcdif: endpoint@0 {
reg = <0>;
remote-endpoint = <&lcdif_out_dsi>;
};
};
port@1 {
reg = <1>;
dsi_out_bridge: endpoint {
remote-endpoint = <&bridge_in_dsi>;
};
};
};
The child node interface represents the child devices via
conventional child nodes on given DSI parent like
dsi {
compatible = "samsung,exynos5433-mipi-dsi";
ports {
port@0 {
reg = <0>;
dsi_to_mic: endpoint {
remote-endpoint = <&mic_to_dsi>;
};
};
};
panel@0 {
reg = <0>;
};
};
As Samsung DSIM bridge is common DSI IP across all Exynos DSI
and NXP i.MX8M host controllers, this patch adds support to
lookup the child devices whether its bindings on the associated
host represent OF-graph or child node interfaces.
v6, v5, v4, v3:
* none
v2:
* new patch
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 38 +--
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 73dcd825c654..e41b6eeef622 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1356,18 +1356,52 @@ static int samsung_dsim_host_attach(struct
mipi_dsi_host *host,
struct samsung_dsim *dsi = host_to_dsi(host);
const struct samsung_dsim_plat_data *pdata = dsi->plat_data;
struct device *dev = dsi->dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *remote;
struct drm_panel *panel;
int ret;
- panel = of_drm_find_panel(device->dev.of_node);
+ /**
+* Devices can also be child nodes when we also control that device
+* through the upstream device (ie, MIPI-DCS for a MIPI-DSI device).
+*
+* Lookup for a child node of the given parent that isn't either port
+* or ports.
+*/
+ for_each_available_child_of_node(np, remote) {
+ if (of_node_name_eq(remote, "port") ||
+ of_node_name_eq(remote, "ports"))
+ continue;
+
+ goto of_find_panel_or_bridge;
+ }
+
+ /*
+* of_graph_get_remote_node() produces a noisy error message if port
+* node isn't found and the absence of the port is a legit case here,
+* so at first we silently check whether graph presents in the
+* device-tree node.
+*/
+ if (!of_graph_is_present(np))
+ return -ENODEV;
+
+ remote = of_graph_get_remote_node(np, 1, 0);
+
+of_find_panel_or_bridge:
+ if (!remote)
+ return -ENODEV;
+
+ panel = of_drm_find_panel(remote);
if (!IS_ERR(panel)) {
dsi->out_bridge = devm_drm_panel_bridge_add(dev, panel);
} else {
- dsi->out_bridge = of_drm_find_bridge(device->dev.of_node);
+ dsi->out_bridge = of_drm_find_bridge(remote);
if (!dsi->out_bridge)
dsi->out_bridge = ERR_PTR(-EINVAL);
}
+ of_node_put(remote);
+
if (IS_ERR(dsi->out_bridge)) {
ret = PTR_ERR(dsi->out_bridge);
DRM_DEV_ERROR(dev, "failed to find the bridge: %d\n", ret);
--
2.25.1
[PATCH v6 03/10] drm: bridge: samsung-dsim: Mark PHY as optional
In i.MX8M Mini/Nano SoC the DSI Phy requires a MIPI DPHY bit
to reset in order to activate the PHY and that can be done via
upstream i.MX8M blk-ctrl driver.
So, mark the phy get as optional.
v6, v5, v4, v3, v2:
* none
v1:
* new patch
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index e41b6eeef622..2ba909ec5239 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1584,7 +1584,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
if (IS_ERR(dsi->reg_base))
return PTR_ERR(dsi->reg_base);
- dsi->phy = devm_phy_get(dev, "dsim");
+ dsi->phy = devm_phy_optional_get(dev, "dsim");
if (IS_ERR(dsi->phy)) {
dev_info(dev, "failed to get dsim phy\n");
return PTR_ERR(dsi->phy);
--
2.25.1
[PATCH v6 04/10] drm: bridge: samsung-dsim: Handle proper DSI host initialization
DSI host initialization handling in previous exynos dsi driver has
some pitfalls. It initializes the host during host transfer() hook
that is indeed not the desired call flow for I2C and any other DSI
configured downstream bridges.
Host transfer() is usually triggered for downstream DSI panels or
bridges and I2C-configured-DSI bridges miss these host initialization
as these downstream bridges use bridge operations hooks like pre_enable,
and enable in order to initialize or set up the host.
This patch is trying to handle the host init handler to satisfy all
downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED state
flag to ensure that host init is also done on first cmd transfer, this
helps existing DSI panels work on exynos platform (form Marek
Szyprowski).
v6, v5:
* none
v4:
* update init handling to ensure host init done on first cmd transfer
v3:
* none
v2:
* check initialized state in samsung_dsim_init
v1:
* keep DSI init in host transfer
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 25 +
include/drm/bridge/samsung-dsim.h | 5 +++--
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 2ba909ec5239..0636440e4420 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1234,12 +1234,17 @@ static void samsung_dsim_disable_irq(struct
samsung_dsim *dsi)
disable_irq(dsi->irq);
}
-static int samsung_dsim_init(struct samsung_dsim *dsi)
+static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned int flag)
{
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
+ if (dsi->state & flag)
+ return 0;
+
samsung_dsim_reset(dsi);
- samsung_dsim_enable_irq(dsi);
+
+ if (!(dsi->state & DSIM_STATE_INITIALIZED))
+ samsung_dsim_enable_irq(dsi);
if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
@@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi)
samsung_dsim_set_phy_ctrl(dsi);
samsung_dsim_init_link(dsi);
+ dsi->state |= flag;
+
return 0;
}
@@ -1269,6 +1276,10 @@ static void samsung_dsim_atomic_pre_enable(struct
drm_bridge *bridge,
}
dsi->state |= DSIM_STATE_ENABLED;
+
+ ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
+ if (ret)
+ return;
}
static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
@@ -1458,12 +1469,9 @@ static ssize_t samsung_dsim_host_transfer(struct
mipi_dsi_host *host,
if (!(dsi->state & DSIM_STATE_ENABLED))
return -EINVAL;
- if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
- ret = samsung_dsim_init(dsi);
- if (ret)
- return ret;
- dsi->state |= DSIM_STATE_INITIALIZED;
- }
+ ret = samsung_dsim_init(dsi, DSIM_STATE_REINITIALIZED);
+ if (ret)
+ return ret;
ret = mipi_dsi_create_packet(&xfer.packet, msg);
if (ret < 0)
@@ -1653,6 +1661,7 @@ static int __maybe_unused samsung_dsim_suspend(struct
device *dev)
if (dsi->state & DSIM_STATE_INITIALIZED) {
dsi->state &= ~DSIM_STATE_INITIALIZED;
+ dsi->state &= ~DSIM_STATE_REINITIALIZED;
samsung_dsim_disable_clock(dsi);
diff --git a/include/drm/bridge/samsung-dsim.h
b/include/drm/bridge/samsung-dsim.h
index b8132bf8e36f..0c5a905f3de7 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -17,8 +17,9 @@ struct samsung_dsim;
#define DSIM_STATE_ENABLED BIT(0)
#define DSIM_STATE_INITIALIZED BIT(1)
-#define DSIM_STATE_CMD_LPM BIT(2)
-#define DSIM_STATE_VIDOUT_AVAILABLEBIT(3)
+#define DSIM_STATE_REINITIALIZED BIT(2)
+#define DSIM_STATE_CMD_LPM BIT(3)
+#define DSIM_STATE_VIDOUT_AVAILABLEBIT(4)
enum samsung_dsim_type {
SAMSUNG_DSIM_TYPE_EXYNOS3250,
--
2.25.1
[PATCH v6 05/10] drm: bridge: samsung-dsim: Add atomic_check
Look like an explicit fixing up of mode_flags is required for DSIM IP
present in i.MX8M Mini/Nano SoCs.
At least the LCDIF + DSIM needs active low sync polarities in order
to correlate the correct sync flags of the surrounding components in
the chain to make sure the whole pipeline can work properly.
On the other hand the i.MX 8M Mini Applications Processor Reference Manual,
Rev. 3, 11/2020 says.
"13.6.3.5.2 RGB interface
Vsync, Hsync, and VDEN are active high signals."
No clear evidence about whether it can be documentation issues or
something, so added a comment FIXME for this and updated the active low
sync polarities using SAMSUNG_DSIM_TYPE_IMX8MM hw_type.
v6:
* none
v5:
* rebase based new bridge changes [mszyprow]
* remove DSIM_QUIRK_FIXUP_SYNC_POL
* add hw_type check for sync polarities change.
v4:
* none
v3:
* add DSIM_QUIRK_FIXUP_SYNC_POL to handle mode_flasg fixup
v2:
* none
v1:
* fix mode flags in atomic_check instead of mode_fixup
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 0636440e4420..90506be3f2dd 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1315,6 +1315,31 @@ static void samsung_dsim_atomic_post_disable(struct
drm_bridge *bridge,
pm_runtime_put_sync(dsi->dev);
}
+static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
+struct drm_bridge_state *bridge_state,
+struct drm_crtc_state *crtc_state,
+struct drm_connector_state *conn_state)
+{
+ struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+ struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
+
+ if (dsi->plat_data->hw_type & SAMSUNG_DSIM_TYPE_IMX8MM) {
+ /**
+* FIXME:
+* At least LCDIF + DSIM needs active low sync,
+* but i.MX 8M Mini Applications Processor Reference Manual,
+* Rev. 3, 11/2020 says
+*
+* 13.6.3.5.2 RGB interface
+* Vsync, Hsync, and VDEN are active high signals.
+*/
+ adjusted_mode->flags |= (DRM_MODE_FLAG_NHSYNC |
DRM_MODE_FLAG_NVSYNC);
+ adjusted_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC |
DRM_MODE_FLAG_PVSYNC);
+ }
+
+ return 0;
+}
+
static void samsung_dsim_mode_set(struct drm_bridge *bridge,
const struct drm_display_mode *mode,
const struct drm_display_mode *adjusted_mode)
@@ -1353,6 +1378,7 @@ static const struct drm_bridge_funcs
samsung_dsim_bridge_funcs = {
.atomic_duplicate_state =
drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state =
drm_atomic_helper_bridge_destroy_state,
.atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_check = samsung_dsim_atomic_check,
.atomic_pre_enable = samsung_dsim_atomic_pre_enable,
.atomic_enable = samsung_dsim_atomic_enable,
.atomic_disable = samsung_dsim_atomic_disable,
--
2.25.1
[PATCH v6 06/10] drm: bridge: samsung-dsim: Add platform PLL_P (PMS_P) offset
Look like PLL PMS_P offset value varies between platforms that have
Samsung DSIM IP.
However, there is no clear evidence for it as both Exynos and i.MX
8M Mini Application Processor Reference Manual is still referring
the PMS_P offset as 13.
The offset 13 is not working for i.MX8M Mini SoCs but the downstream
NXP sec-dsim.c driver is using offset 14 for i.MX8M Mini SoC platforms
[1] [2].
PMS_P value set in sec_mipi_dsim_check_pll_out using PLLCTRL_SET_P()
with offset 13 and then an additional offset of one bit added in
sec_mipi_dsim_config_pll via PLLCTRL_SET_PMS().
Not sure whether it is reference manual documentation or something else
but this patch trusts the downstream code and handle PLL_P offset via
platform driver data so-that imx8mm driver data shall use pll_p_offset
to 14.
[1]
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n210
[2]
https://source.codeaurora.org/external/imx/linux-imx/tree/drivers/gpu/drm/bridge/sec-dsim.c?h=imx_5.4.47_2.2.0#n211
v6:
* none
v5:
* updated clear commit message
v4, v3, v2:
* none
v1:
* updated commit message
* add downstream driver link
Signed-off-by: Frieder Schrempf
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 10 --
include/drm/bridge/samsung-dsim.h | 1 +
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 90506be3f2dd..d0bb96a275fd 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -168,7 +168,7 @@
/* DSIM_PLLCTRL */
#define DSIM_FREQ_BAND(x) ((x) << 24)
#define DSIM_PLL_EN(1 << 23)
-#define DSIM_PLL_P(x) ((x) << 13)
+#define DSIM_PLL_P(x, offset) ((x) << (offset))
#define DSIM_PLL_M(x) ((x) << 4)
#define DSIM_PLL_S(x) ((x) << 1)
@@ -368,6 +368,7 @@ static const struct samsung_dsim_driver_data
exynos3_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+ .pll_p_offset = 13,
.reg_values = reg_values,
};
@@ -380,6 +381,7 @@ static const struct samsung_dsim_driver_data
exynos4_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+ .pll_p_offset = 13,
.reg_values = reg_values,
};
@@ -390,6 +392,7 @@ static const struct samsung_dsim_driver_data
exynos5_dsi_driver_data = {
.max_freq = 1000,
.wait_for_reset = 1,
.num_bits_resol = 11,
+ .pll_p_offset = 13,
.reg_values = reg_values,
};
@@ -401,6 +404,7 @@ static const struct samsung_dsim_driver_data
exynos5433_dsi_driver_data = {
.max_freq = 1500,
.wait_for_reset = 0,
.num_bits_resol = 12,
+ .pll_p_offset = 13,
.reg_values = exynos5433_reg_values,
};
@@ -412,6 +416,7 @@ static const struct samsung_dsim_driver_data
exynos5422_dsi_driver_data = {
.max_freq = 1500,
.wait_for_reset = 1,
.num_bits_resol = 12,
+ .pll_p_offset = 13,
.reg_values = exynos5422_reg_values,
};
@@ -543,7 +548,8 @@ static unsigned long samsung_dsim_set_pll(struct
samsung_dsim *dsi,
writel(driver_data->reg_values[PLL_TIMER],
dsi->reg_base + driver_data->plltmr_reg);
- reg = DSIM_PLL_EN | DSIM_PLL_P(p) | DSIM_PLL_M(m) | DSIM_PLL_S(s);
+ reg = DSIM_PLL_EN | DSIM_PLL_P(p, driver_data->pll_p_offset) |
+ DSIM_PLL_M(m) | DSIM_PLL_S(s);
if (driver_data->has_freqband) {
static const unsigned long freq_bands[] = {
diff --git a/include/drm/bridge/samsung-dsim.h
b/include/drm/bridge/samsung-dsim.h
index 0c5a905f3de7..df3d030daec6 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -53,6 +53,7 @@ struct samsung_dsim_driver_data {
unsigned int max_freq;
unsigned int wait_for_reset;
unsigned int num_bits_resol;
+ unsigned int pll_p_offset;
const unsigned int *reg_values;
};
--
2.25.1
[PATCH v6 07/10] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts
Finding the right input bus format throughout the pipeline is hard
so add atomic_get_input_bus_fmts callback and initialize with the
default RGB888_1X24 bus format on DSI-end.
This format can be used in pipeline for negotiating bus format between
the DSI-end of this bridge and the other component closer to pipeline
components.
v6, v5, v4:
* none
v3:
* include media-bus-format.h
v2:
* none
v1:
* new patch
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 28 +++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index d0bb96a275fd..4fd77172bb4b 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct
drm_bridge *bridge,
pm_runtime_put_sync(dsi->dev);
}
+#define MAX_INPUT_SEL_FORMATS 1
+
+static u32 *
+samsung_dsim_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state,
+ u32 output_fmt,
+ unsigned int *num_input_fmts)
+{
+ u32 *input_fmts;
+
+ *num_input_fmts = 0;
+
+ input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+GFP_KERNEL);
+ if (!input_fmts)
+ return NULL;
+
+ /* This is the DSI-end bus format */
+ input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+ *num_input_fmts = 1;
+
+ return input_fmts;
+}
+
static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
struct drm_bridge_state *bridge_state,
struct drm_crtc_state *crtc_state,
@@ -1384,6 +1411,7 @@ static const struct drm_bridge_funcs
samsung_dsim_bridge_funcs = {
.atomic_duplicate_state =
drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state =
drm_atomic_helper_bridge_destroy_state,
.atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_get_input_bus_fmts =
samsung_dsim_atomic_get_input_bus_fmts,
.atomic_check = samsung_dsim_atomic_check,
.atomic_pre_enable = samsung_dsim_atomic_pre_enable,
.atomic_enable = samsung_dsim_atomic_enable,
--
2.25.1
[PATCH v6 08/10] drm: bridge: samsung-dsim: Add input_bus_flags
eLCDIF is expecting to have input_bus_flags as DE_LOW in order to
set active low during valid data transfer on each horizontal line.
Add DE_LOW flag via drm bridge timings.
v6:
* none
v5:
* rebased based on updated bridge changes
v4, v3, v2, v1:
* none
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 4fd77172bb4b..49406a07d655 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1601,6 +1601,10 @@ static const struct samsung_dsim_host_ops
samsung_dsim_generic_host_ops = {
.unregister_host = samsung_dsim_unregister_host,
};
+static const struct drm_bridge_timings samsung_dsim_bridge_timings = {
+ .input_bus_flags = DRM_BUS_FLAG_DE_LOW,
+};
+
int samsung_dsim_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1681,6 +1685,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
dsi->bridge.funcs = &samsung_dsim_bridge_funcs;
dsi->bridge.of_node = dev->of_node;
+ dsi->bridge.timings = &samsung_dsim_bridge_timings;
dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
if (dsi->plat_data->host_ops && dsi->plat_data->host_ops->register_host)
--
2.25.1
[PATCH v6 09/10] dt-bindings: display: exynos: dsim: Add NXP i.MX8MM support
Samsung MIPI DSIM bridge can also be found in i.MX8MM SoC. Add dt-bingings for it. v6, v5, v4: * none v3: * collect Rob Acked-by v2: * updated comments v1: * new patch Acked-by: Rob Herring Signed-off-by: Jagan Teki --- Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt index be377786e8cd..8efcf4728e0b 100644 --- a/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt +++ b/Documentation/devicetree/bindings/display/exynos/exynos_dsim.txt @@ -7,6 +7,7 @@ Required properties: "samsung,exynos5410-mipi-dsi" /* for Exynos5410/5420/5440 SoCs */ "samsung,exynos5422-mipi-dsi" /* for Exynos5422/5800 SoCs */ "samsung,exynos5433-mipi-dsi" /* for Exynos5433 SoCs */ + "fsl,imx8mm-mipi-dsim" /* for i.MX8M Mini SoCs */ - reg: physical base address and length of the registers set for the device - interrupts: should contain DSI interrupt - clocks: list of clock specifiers, must contain an entry for each required -- 2.25.1
[PATCH v6 10/10] drm: bridge: samsung-dsim: Add i.MX8MM support
Samsung MIPI DSIM master can also be found in i.MX8MM SoC.
Add compatible and associated driver_data for it.
v6:
* none
v3:
* enable DSIM_QUIRK_FIXUP_SYNC_POL quirk
v5:
* [mszyprow] rebased and adjusted to the new driver initialization
* drop quirk
v4:
* none
v3:
* enable DSIM_QUIRK_FIXUP_SYNC_POL quirk
v2:
* collect Laurent r-b
v1:
* none
Reviewed-by: Laurent Pinchart
Signed-off-by: Marek Szyprowski
Signed-off-by: Jagan Teki
---
drivers/gpu/drm/bridge/samsung-dsim.c | 45 +++
1 file changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 49406a07d655..5f2c51428cdd 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -360,6 +360,24 @@ static const unsigned int exynos5433_reg_values[] = {
[PHYTIMING_HS_TRAIL] = DSIM_PHYTIMING2_HS_TRAIL(0x0c),
};
+static const unsigned int imx8mm_dsim_reg_values[] = {
+ [RESET_TYPE] = DSIM_SWRST,
+ [PLL_TIMER] = 500,
+ [STOP_STATE_CNT] = 0xf,
+ [PHYCTRL_ULPS_EXIT] = 0,
+ [PHYCTRL_VREG_LP] = 0,
+ [PHYCTRL_SLEW_UP] = 0,
+ [PHYTIMING_LPX] = DSIM_PHYTIMING_LPX(0x06),
+ [PHYTIMING_HS_EXIT] = DSIM_PHYTIMING_HS_EXIT(0x0b),
+ [PHYTIMING_CLK_PREPARE] = DSIM_PHYTIMING1_CLK_PREPARE(0x07),
+ [PHYTIMING_CLK_ZERO] = DSIM_PHYTIMING1_CLK_ZERO(0x26),
+ [PHYTIMING_CLK_POST] = DSIM_PHYTIMING1_CLK_POST(0x0d),
+ [PHYTIMING_CLK_TRAIL] = DSIM_PHYTIMING1_CLK_TRAIL(0x08),
+ [PHYTIMING_HS_PREPARE] = DSIM_PHYTIMING2_HS_PREPARE(0x08),
+ [PHYTIMING_HS_ZERO] = DSIM_PHYTIMING2_HS_ZERO(0x0d),
+ [PHYTIMING_HS_TRAIL] = DSIM_PHYTIMING2_HS_TRAIL(0x0b),
+};
+
static const struct samsung_dsim_driver_data exynos3_dsi_driver_data = {
.reg_ofs = exynos_reg_ofs,
.plltmr_reg = 0x50,
@@ -421,6 +439,23 @@ static const struct samsung_dsim_driver_data
exynos5422_dsi_driver_data = {
.reg_values = exynos5422_reg_values,
};
+static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
+ .reg_ofs = exynos5433_reg_ofs,
+ .plltmr_reg = 0xa0,
+ .has_clklane_stop = 1,
+ .num_clks = 2,
+ .max_freq = 2100,
+ .wait_for_reset = 0,
+ .num_bits_resol = 12,
+ /**
+* FIXME:
+* Offset value used from downstream drivers/gpu/drm/bridge/sec-dsim.c
+* remove this comment if it is true else update the logic.
+*/
+ .pll_p_offset = 14,
+ .reg_values = imx8mm_dsim_reg_values,
+};
+
static const struct samsung_dsim_driver_data *
samsung_dsim_types[SAMSUNG_DSIM_TYPE_COUNT] = {
[SAMSUNG_DSIM_TYPE_EXYNOS3250] = &exynos3_dsi_driver_data,
@@ -428,6 +463,7 @@ samsung_dsim_types[SAMSUNG_DSIM_TYPE_COUNT] = {
[SAMSUNG_DSIM_TYPE_EXYNOS5410] = &exynos5_dsi_driver_data,
[SAMSUNG_DSIM_TYPE_EXYNOS5422] = &exynos5422_dsi_driver_data,
[SAMSUNG_DSIM_TYPE_EXYNOS5433] = &exynos5433_dsi_driver_data,
+ [SAMSUNG_DSIM_TYPE_IMX8MM] = &imx8mm_dsi_driver_data,
};
static inline struct samsung_dsim *host_to_dsi(struct mipi_dsi_host *h)
@@ -1788,7 +1824,16 @@ const struct dev_pm_ops samsung_dsim_pm_ops = {
};
EXPORT_SYMBOL_GPL(samsung_dsim_pm_ops);
+static const struct samsung_dsim_plat_data samsung_dsim_imx8mm_pdata = {
+ .hw_type = SAMSUNG_DSIM_TYPE_IMX8MM,
+ .host_ops = &samsung_dsim_generic_host_ops,
+};
+
static const struct of_device_id samsung_dsim_of_match[] = {
+ {
+ .compatible = "fsl,imx8mm-mipi-dsim",
+ .data = &samsung_dsim_imx8mm_pdata,
+ },
{ /* sentinel. */ }
};
MODULE_DEVICE_TABLE(of, samsung_dsim_of_match);
--
2.25.1
Re: [PATCH 4/4] arm64: dts: smaug: Add display panel node
On 30/09/2022 23:14, Rob Herring wrote:
> + dc@5420 {
> + status = "okay";
You should override by labels, not by full path.
>>>
>>> Why exactly is that? I've always stayed away from that (and asked others
>>> not to do so, at least on Tegra) because I find it impossible to parse
>>> for my human brain. Replicating the original full hierarchy makes it
>>> much more obvious to me where the changes are happening than the
>>> spaghetti-like mess that you get from overriding by label reference.
>>
>> Sure, it's entirely up to you. I forgot your preference.
>>
>> But it is a really nice way to have duplicated nodes and mistakes (which
>> happen from time to time).
>
> We could have a schema or dtc check for that. We already warn for
> duplicate unit-addresses which would catch some typos. Checking for a
> node with only 'status' would probably work when that's the only
> addition. Maybe status without a compatible would be better? We also
> check for nodes without a specific schema, but child nodes in schemas
> aren't handled.
Usually these are overrides of few properties and status=okay, so
looking for nodes without a compatible would work. Except for all the
cases where we do not have schema yet...
Best regards,
Krzysztof
Re: [PATCH v6 00/10] drm: bridge: Add Samsung MIPI DSIM bridge
Hi Jagan, On Sat, Oct 1, 2022 at 5:07 AM Jagan Teki wrote: > Repo: > https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v6 This URL returns an error. Please double-check.
Re: [PATCH v4 30/30] drm/sun4i: tv: Convert to the new TV mode property
Den 29.09.2022 18.31, skrev Maxime Ripard:
> Now that the core can deal fine with analog TV modes, let's convert the
> sun4i TV driver to leverage those new features.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/sun4i/sun4i_tv.c | 148
> ++-
> drivers/gpu/drm/vc4/vc4_vec.c| 5 +-
> 2 files changed, 54 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c
> b/drivers/gpu/drm/sun4i/sun4i_tv.c
> @@ -467,35 +398,46 @@ static const struct drm_encoder_helper_funcs
> sun4i_tv_helper_funcs = {
>
> static int sun4i_tv_comp_get_modes(struct drm_connector *connector)
> {
> - int i;
> + struct drm_display_mode *mode;
> + int count = 0;
>
> - for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
> - struct drm_display_mode *mode;
> - const struct tv_mode *tv_mode = &tv_modes[i];
> -
> - mode = drm_mode_create(connector->dev);
> - if (!mode) {
> - DRM_ERROR("Failed to create a new display mode\n");
> - return 0;
> - }
> + mode = drm_mode_analog_ntsc_480i(connector->dev);
> + if (!mode) {
> + DRM_ERROR("Failed to create a new display mode\n");
> + return -ENOMEM;
> + }
>
> - strcpy(mode->name, tv_mode->name);
> + mode->type |= DRM_MODE_TYPE_PREFERRED;
> + drm_mode_probed_add(connector, mode);
> + count += 1;
>
> - sun4i_tv_mode_to_drm_mode(tv_mode, mode);
> - drm_mode_probed_add(connector, mode);
> + mode = drm_mode_analog_pal_576i(connector->dev);
> + if (!mode) {
> + DRM_ERROR("Failed to create a new display mode\n");
> + return -ENOMEM;
> }
>
> - return i;
> + drm_mode_probed_add(connector, mode);
> + count += 1;
> +
> + return count;
count is always 2 so you can just return 2.
Acked-by: Noralf Trønnes
> }
This stray hunk belongs to the vc4 TV mode patch I guess:
> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> index 8d37d7ba9b2a..88b4330bfa39 100644
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -322,7 +322,7 @@ vc4_vec_tv_mode_lookup(unsigned int mode)
> return NULL;
> }
>
> -static const struct drm_prop_enum_list tv_mode_names[] = {
> +static const struct drm_prop_enum_list legacy_tv_mode_names[] = {
> { VC4_VEC_TV_MODE_NTSC, "NTSC", },
> { VC4_VEC_TV_MODE_NTSC_443, "NTSC-443", },
> { VC4_VEC_TV_MODE_NTSC_J, "NTSC-J", },
> @@ -498,7 +498,8 @@ static int vc4_vec_connector_init(struct drm_device *dev,
> struct vc4_vec *vec)
> DRM_MODE_TV_MODE_NTSC);
>
> prop = drm_property_create_enum(dev, 0, "mode",
> - tv_mode_names,
> ARRAY_SIZE(tv_mode_names));
> + legacy_tv_mode_names,
> + ARRAY_SIZE(legacy_tv_mode_names));
> if (!prop)
> return -ENOMEM;
> vec->legacy_tv_mode_property = prop;
>
Noralf.
Re: [PATCH v4 11/30] drm/modes: Add a function to generate analog display modes
Den 29.09.2022 18.31, skrev Maxime Ripard:
> Multiple drivers (meson, vc4, sun4i) define analog TV 525-lines and
> 625-lines modes in their drivers.
>
> Since those modes are fairly standard, and that we'll need to use them
> in more places in the future, it makes sense to move their definition
> into the core framework.
>
> However, analog display usually have fairly loose timings requirements,
> the only discrete parameters being the total number of lines and pixel
> clock frequency. Thus, we created a function that will create a display
> mode from the standard, the pixel frequency and the active area.
>
> Signed-off-by: Maxime Ripard
>
> ---
>
> Changes in v4:
> - Reworded the line length check comment
> - Switch to HZ_PER_KHZ in tests
> - Use previous timing to fill our mode
> - Move the number of lines check earlier
> ---
> drivers/gpu/drm/drm_modes.c| 474
> +
> drivers/gpu/drm/tests/Makefile | 1 +
> drivers/gpu/drm/tests/drm_modes_test.c | 144 ++
> include/drm/drm_modes.h| 17 ++
> 4 files changed, 636 insertions(+)
>
I haven't followed the discussion on this patch, but it seems rather
excessive to add over 600 lines of code (including tests) to add 2 fixed
modes. And it's very difficult to see from the code what the actual
display mode timings really are, which would be useful for other
developers down the road wanting to use them.
Why not just hardcode the modes?
Noralf.
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 304004fb80aa..7cf2fe98d7d2 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -116,6 +116,480 @@ void drm_mode_probed_add(struct drm_connector
> *connector,
> }
> EXPORT_SYMBOL(drm_mode_probed_add);
>
> +enum drm_mode_analog {
> + DRM_MODE_ANALOG_NTSC, /* 525 lines, 60Hz */
> + DRM_MODE_ANALOG_PAL, /* 625 lines, 50Hz */
> +};
> +
> +/*
> + * The timings come from:
> + * -
> https://web.archive.org/web/20220406232708/http://www.kolumbus.fi/pami1/video/pal_ntsc.html
> + * -
> https://web.archive.org/web/20220406124914/http://martin.hinner.info/vga/pal.html
> + * -
> https://web.archive.org/web/20220609202433/http://www.batsocks.co.uk/readme/video_timing.htm
> + */
> +#define NTSC_LINE_DURATION_NS63556U
> +#define NTSC_LINES_NUMBER525
> +
> +#define NTSC_HBLK_DURATION_TYP_NS10900U
> +#define NTSC_HBLK_DURATION_MIN_NS(NTSC_HBLK_DURATION_TYP_NS - 200)
> +#define NTSC_HBLK_DURATION_MAX_NS(NTSC_HBLK_DURATION_TYP_NS + 200)
> +
> +#define NTSC_HACT_DURATION_TYP_NS(NTSC_LINE_DURATION_NS -
> NTSC_HBLK_DURATION_TYP_NS)
> +#define NTSC_HACT_DURATION_MIN_NS(NTSC_LINE_DURATION_NS -
> NTSC_HBLK_DURATION_MAX_NS)
> +#define NTSC_HACT_DURATION_MAX_NS(NTSC_LINE_DURATION_NS -
> NTSC_HBLK_DURATION_MIN_NS)
> +
> +#define NTSC_HFP_DURATION_TYP_NS 1500
> +#define NTSC_HFP_DURATION_MIN_NS 1270
> +#define NTSC_HFP_DURATION_MAX_NS 2220
> +
> +#define NTSC_HSLEN_DURATION_TYP_NS 4700
> +#define NTSC_HSLEN_DURATION_MIN_NS (NTSC_HSLEN_DURATION_TYP_NS - 100)
> +#define NTSC_HSLEN_DURATION_MAX_NS (NTSC_HSLEN_DURATION_TYP_NS + 100)
> +
> +#define NTSC_HBP_DURATION_TYP_NS 4700
> +
> +/*
> + * I couldn't find the actual tolerance for the back porch, so let's
> + * just reuse the sync length ones.
> + */
> +#define NTSC_HBP_DURATION_MIN_NS (NTSC_HBP_DURATION_TYP_NS - 100)
> +#define NTSC_HBP_DURATION_MAX_NS (NTSC_HBP_DURATION_TYP_NS + 100)
> +
> +#define PAL_LINE_DURATION_NS 64000U
> +#define PAL_LINES_NUMBER 625
> +
> +#define PAL_HACT_DURATION_TYP_NS 51950U
> +#define PAL_HACT_DURATION_MIN_NS (PAL_HACT_DURATION_TYP_NS - 100)
> +#define PAL_HACT_DURATION_MAX_NS (PAL_HACT_DURATION_TYP_NS + 400)
> +
> +#define PAL_HBLK_DURATION_TYP_NS (PAL_LINE_DURATION_NS -
> PAL_HACT_DURATION_TYP_NS)
> +#define PAL_HBLK_DURATION_MIN_NS (PAL_LINE_DURATION_NS -
> PAL_HACT_DURATION_MAX_NS)
> +#define PAL_HBLK_DURATION_MAX_NS (PAL_LINE_DURATION_NS -
> PAL_HACT_DURATION_MIN_NS)
> +
> +#define PAL_HFP_DURATION_TYP_NS 1650
> +#define PAL_HFP_DURATION_MIN_NS (PAL_HFP_DURATION_TYP_NS - 100)
> +#define PAL_HFP_DURATION_MAX_NS (PAL_HFP_DURATION_TYP_NS + 400)
> +
> +#define PAL_HSLEN_DURATION_TYP_NS4700
> +#define PAL_HSLEN_DURATION_MIN_NS(PAL_HSLEN_DURATION_TYP_NS - 200)
> +#define PAL_HSLEN_DURATION_MAX_NS(PAL_HSLEN_DURATION_TYP_NS + 200)
> +
> +#define PAL_HBP_DURATION_TYP_NS 5700
> +#define PAL_HBP_DURATION_MIN_NS (PAL_HBP_DURATION_TYP_NS - 200)
> +#define PAL_HBP_DURATION_MAX_NS (PAL_HBP_DURATION_TYP_NS + 200)
> +
> +struct analog_param_field {
> + unsigned int even, odd;
> +};
> +
> +#define PARAM_FIELD(_odd, _even) \
> + { .even = _even, .odd = _odd }
> +
> +struct analog_param_range {
> + unsigned intmin, typ, max;
> +
Re: [PATCH v4 00/30] drm: Analog TV Improvements
Den 29.09.2022 18.30, skrev Maxime Ripard: > Hi, > > Here's a series aiming at improving the command line named modes support, > and more importantly how we deal with all the analog TV variants. > > The named modes support were initially introduced to allow to specify the > analog TV mode to be used. > > However, this was causing multiple issues: > > * The mode name parsed on the command line was passed directly to the > driver, which had to figure out which mode it was suppose to match; > > * Figuring that out wasn't really easy, since the video= argument or what > the userspace might not even have a name in the first place, but > instead could have passed a mode with the same timings; > > * The fallback to matching on the timings was mostly working as long as > we were supporting one 525 lines (most likely NSTC) and one 625 lines > (PAL), but couldn't differentiate between two modes with the same > timings (NTSC vs PAL-M vs NSTC-J for example); > > * There was also some overlap with the tv mode property registered by > drm_mode_create_tv_properties(), but named modes weren't interacting > with that property at all. > > * Even though that property was generic, its possible values were > specific to each drivers, which made some generic support difficult. > > Thus, I chose to tackle in multiple steps: > > * A new TV mode property was introduced, with generic values, each driver > reporting through a bitmask what standard it supports to the userspace; > > * This option was added to the command line parsing code to be able to > specify it on the kernel command line, and new atomic_check and reset > helpers were created to integrate properly into atomic KMS; > > * The named mode parsing code is now creating a proper display mode for > the given named mode, and the TV standard will thus be part of the > connector state; > > * Two drivers were converted and tested for now (vc4 and sun4i), with > some backward compatibility code to translate the old TV mode to the > new TV mode; > > Unit tests were created along the way. > > One can switch from NTSC to PAL now using (on vc4) > > modetest -M vc4 -s 53:720x480i -w 53:'TV mode':1 # NTSC > modetest -M vc4 -s 53:720x576i -w 53:'TV mode':4 # PAL > > Let me know what you think, > Maxime > I suggest that you apply the patches that are reviewed, have merrit on their own and are not tied to the TV mode property. This will help in keeping this rather big patchset focused and ease the task for reviewers. The following seems to be in that group: drm/tests: Order Kunit tests in Makefile drm/atomic-helper: Rename drm_atomic_helper_connector_tv_reset to avoid ambiguity drm/connector: Rename subconnector state variable drm/atomic: Add TV subconnector property to get/set_property drm/modes: Only consider bpp and refresh before options drm/modes: parse_cmdline: Add support for named modes containing dashes drm/vc4: vec: Fix definition of PAL-M mode Noralf.
Re: [v5] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280
On 29 September 2022 19:13:20 GMT+03:00, Doug Anderson wrote: >Hi, > >On Wed, Sep 14, 2022 at 5:16 AM Kalyan Thota wrote: >> >> Flush mechanism for DSPP blocks has changed in sc7280 family, it >> allows individual sub blocks to be flushed in coordination with >> master flush control. >> >> Representation: master_flush && (PCC_flush | IGC_flush .. etc ) >> >> This change adds necessary support for the above design. >> >> Changes in v1: >> - Few nits (Doug, Dmitry) >> - Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry) >> >> Changes in v2: >> - Move the address offset to flush macro (Dmitry) >> - Seperate ops for the sub block flush (Dmitry) >> >> Changes in v3: >> - Reuse the DPU_DSPP_xx enum instead of a new one (Dmitry) >> >> Changes in v4: >> - Use shorter version for unsigned int (Stephen) >> >> Reviewed-by: Dmitry Baryshkov >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 >> -- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++-- >> 5 files changed, 50 insertions(+), 6 deletions(-) > >Breadcrumbs: though this is tagged in the subject as v5 I think the >newest version is actually "resend v4" [1] which just fixes the >Signed-off-by. Not to mention that v5 misses the S-o-B tag. > >[1] >https://lore.kernel.org/r/1663825463-6715-1-git-send-email-quic_kaly...@quicinc.com -- With best wishes Dmitry
Re: [v5] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280
On 29 September 2022 19:13:20 GMT+03:00, Doug Anderson wrote: >Hi, > >On Wed, Sep 14, 2022 at 5:16 AM Kalyan Thota wrote: >> >> Flush mechanism for DSPP blocks has changed in sc7280 family, it >> allows individual sub blocks to be flushed in coordination with >> master flush control. >> >> Representation: master_flush && (PCC_flush | IGC_flush .. etc ) >> >> This change adds necessary support for the above design. >> >> Changes in v1: >> - Few nits (Doug, Dmitry) >> - Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry) >> >> Changes in v2: >> - Move the address offset to flush macro (Dmitry) >> - Seperate ops for the sub block flush (Dmitry) >> >> Changes in v3: >> - Reuse the DPU_DSPP_xx enum instead of a new one (Dmitry) >> >> Changes in v4: >> - Use shorter version for unsigned int (Stephen) >> >> Reviewed-by: Dmitry Baryshkov >> --- >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 >> -- >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++-- >> 5 files changed, 50 insertions(+), 6 deletions(-) > >Breadcrumbs: though this is tagged in the subject as v5 I think the >newest version is actually "resend v4" [1] which just fixes the >Signed-off-by. Not to mention that v5 misses the S-o-B tag. > >[1] >https://lore.kernel.org/r/1663825463-6715-1-git-send-email-quic_kaly...@quicinc.com -- With best wishes Dmitry
Re: [PATCH v6 00/10] drm: bridge: Add Samsung MIPI DSIM bridge
Hi Fabio, On Sat, Oct 1, 2022 at 4:04 PM Fabio Estevam wrote: > > Hi Jagan, > > On Sat, Oct 1, 2022 at 5:07 AM Jagan Teki wrote: > > > Repo: > > https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v6 > > This URL returns an error. Please double-check. Thanks for the notice. Now it is accessible. Jagan.
RE: [v5] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280
>-Original Message- >From: Dmitry Baryshkov >Sent: Friday, September 30, 2022 1:59 PM >To: Doug Anderson ; Kalyan Thota (QUIC) > >Cc: y...@qualcomm.com; dri-devel ; linux-arm- >msm ; freedreno >; open list:OPEN FIRMWARE AND FLATTENED >DEVICE TREE BINDINGS ; LKML ker...@vger.kernel.org>; Rob Clark ; Stephen Boyd >; Vinod Polimera (QUIC) >; Abhinav Kumar (QUIC) > >Subject: Re: [v5] drm/msm/disp/dpu1: add support for dspp sub block flush in >sc7280 > >WARNING: This email originated from outside of Qualcomm. Please be wary of >any links or attachments, and do not enable macros. > >On 29 September 2022 19:13:20 GMT+03:00, Doug Anderson > wrote: >>Hi, >> >>On Wed, Sep 14, 2022 at 5:16 AM Kalyan Thota >wrote: >>> >>> Flush mechanism for DSPP blocks has changed in sc7280 family, it >>> allows individual sub blocks to be flushed in coordination with >>> master flush control. >>> >>> Representation: master_flush && (PCC_flush | IGC_flush .. etc ) >>> >>> This change adds necessary support for the above design. >>> >>> Changes in v1: >>> - Few nits (Doug, Dmitry) >>> - Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry) >>> >>> Changes in v2: >>> - Move the address offset to flush macro (Dmitry) >>> - Seperate ops for the sub block flush (Dmitry) >>> >>> Changes in v3: >>> - Reuse the DPU_DSPP_xx enum instead of a new one (Dmitry) >>> >>> Changes in v4: >>> - Use shorter version for unsigned int (Stephen) >>> >>> Reviewed-by: Dmitry Baryshkov >>> --- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 >-- >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++-- >>> 5 files changed, 50 insertions(+), 6 deletions(-) >> >>Breadcrumbs: though this is tagged in the subject as v5 I think the >>newest version is actually "resend v4" [1] which just fixes the >>Signed-off-by. > >Not to mention that v5 misses the S-o-B tag. > >> >>[1] >>https://lore.kernel.org/r/1663825463-6715-1-git-send-email-quic_kalyant >>@quicinc.com > Latest one is https://lore.kernel.org/r/1663825463-6715-1-git-send-email-quic_kaly...@quicinc.com that I last posted. Don’t recollect on why tag was marked as v5. To avoid confusion, shall I resend it again ? >-- >With best wishes >Dmitry
[PATCH] drm: Add 64:27 and 256:135 picture aspect ratio support
Drm driver did not report connector can support 64:27 and 256:135 picture
aspect ratio. Even if drm_edid driver already have those modes in
CEA table. Add both of them then user space application would program
proper picture apsect ratio when HDMI 2.1 monitor connected.
Cc: Shankar Uma
Cc: Ville Syrjälä
Signed-off-by: Lee Shawn C
---
drivers/gpu/drm/drm_connector.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index e3142c8142b3..45078d11c7d3 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -948,6 +948,8 @@ static const struct drm_prop_enum_list
drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_NONE, "Automatic" },
{ DRM_MODE_PICTURE_ASPECT_4_3, "4:3" },
{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
+ { DRM_MODE_PICTURE_ASPECT_64_27, "64:27" },
+ { DRM_MODE_PICTURE_ASPECT_256_135, "256:135" },
};
static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
--
2.31.1
[PATCH drm-misc-next v2 0/9] drm/fsl-dcu: use drm managed resources
Hi,
This patch series converts the driver to use drm managed resources to prevent
potential use-after-free issues on driver unbind/rebind and to get rid of the
usage of deprecated APIs.
Changes in v2:
- While protecting critical sections with drm_dev_{enter,exit} I forgot to
handle alternate return paths within the read-side critical sections, hence
fix them.
- Add a patch to remove explicit calls to drm_mode_config_cleanup() and switch
to drmm_mode_config_init() explicitly.
Danilo Krummrich (9):
drm/fsl-dcu: use drmm_* to allocate driver structures
drm/fsl-dcu: replace drm->dev_private with drm_to_fsl_dcu_drm_dev()
drm/fsl-dcu: crtc: use drmm_crtc_init_with_planes()
drm/fsl-dcu: plane: use drm managed resources
drm/fsl-dcu: use drm_dev_unplug()
drm/fsl-dcu: plane: protect device resources after removal
drm/fsl-dcu: crtc: protect device resources after removal
drm/fsl-dcu: remove trailing return statements
drm/fsl-dcu: remove calls to drm_mode_config_cleanup()
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 64 -
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 47 ++-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 4 +-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 20 ---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 58 ++-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 8 +--
6 files changed, 116 insertions(+), 85 deletions(-)
base-commit: 08fb97de03aa2205c6791301bd83a095abc1949c
--
2.37.3
[PATCH drm-misc-next v2 1/9] drm/fsl-dcu: use drmm_* to allocate driver structures
Use drm managed resources to allocate driver structures and get rid of
the deprecated drm_dev_alloc() call and replace it with
devm_drm_dev_alloc().
This also serves as preparation to get rid of drm_device->dev_private
and to fix use-after-free issues on driver unload.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 7 ++---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 30 +-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 2 +-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 19 +++---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 8 +++---
5 files changed, 31 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 2af60d98f48f..a1b8ce70928a 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -171,15 +171,16 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device
*fsl_dev)
{
struct drm_plane *primary;
struct drm_crtc *crtc = &fsl_dev->crtc;
+ struct drm_device *drm = &fsl_dev->base;
int ret;
- fsl_dcu_drm_init_planes(fsl_dev->drm);
+ fsl_dcu_drm_init_planes(drm);
- primary = fsl_dcu_drm_primary_create_plane(fsl_dev->drm);
+ primary = fsl_dcu_drm_primary_create_plane(drm);
if (!primary)
return -ENOMEM;
- ret = drm_crtc_init_with_planes(fsl_dev->drm, crtc, primary, NULL,
+ ret = drm_crtc_init_with_planes(drm, crtc, primary, NULL,
&fsl_dcu_drm_crtc_funcs, NULL);
if (ret) {
primary->funcs->destroy(primary);
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index b4acc3422ba4..a47b72995fcf 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -168,6 +168,7 @@ static const struct drm_driver fsl_dcu_drm_driver = {
static int fsl_dcu_drm_pm_suspend(struct device *dev)
{
struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
+ struct drm_device *drm = &fsl_dev->base;
int ret;
if (!fsl_dev)
@@ -175,7 +176,7 @@ static int fsl_dcu_drm_pm_suspend(struct device *dev)
disable_irq(fsl_dev->irq);
- ret = drm_mode_config_helper_suspend(fsl_dev->drm);
+ ret = drm_mode_config_helper_suspend(drm);
if (ret) {
enable_irq(fsl_dev->irq);
return ret;
@@ -189,6 +190,7 @@ static int fsl_dcu_drm_pm_suspend(struct device *dev)
static int fsl_dcu_drm_pm_resume(struct device *dev)
{
struct fsl_dcu_drm_device *fsl_dev = dev_get_drvdata(dev);
+ struct drm_device *drm = &fsl_dev->base;
int ret;
if (!fsl_dev)
@@ -202,10 +204,10 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
if (fsl_dev->tcon)
fsl_tcon_bypass_enable(fsl_dev->tcon);
- fsl_dcu_drm_init_planes(fsl_dev->drm);
+ fsl_dcu_drm_init_planes(drm);
enable_irq(fsl_dev->irq);
- drm_mode_config_helper_resume(fsl_dev->drm);
+ drm_mode_config_helper_resume(drm);
return 0;
}
@@ -255,9 +257,10 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
int ret;
u8 div_ratio_shift = 0;
- fsl_dev = devm_kzalloc(dev, sizeof(*fsl_dev), GFP_KERNEL);
- if (!fsl_dev)
- return -ENOMEM;
+ fsl_dev = devm_drm_dev_alloc(dev, &fsl_dcu_drm_driver,
+typeof(*fsl_dev), base);
+ if (IS_ERR(fsl_dev))
+ return PTR_ERR(fsl_dev);
id = of_match_node(fsl_dcu_of_match, pdev->dev.of_node);
if (!id)
@@ -317,28 +320,19 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
fsl_dev->tcon = fsl_tcon_init(dev);
- drm = drm_dev_alloc(&fsl_dcu_drm_driver, dev);
- if (IS_ERR(drm)) {
- ret = PTR_ERR(drm);
- goto unregister_pix_clk;
- }
-
fsl_dev->dev = dev;
- fsl_dev->drm = drm;
fsl_dev->np = dev->of_node;
drm->dev_private = fsl_dev;
dev_set_drvdata(dev, fsl_dev);
ret = drm_dev_register(drm, 0);
if (ret < 0)
- goto put;
+ goto unregister_pix_clk;
drm_fbdev_generic_setup(drm, legacyfb_depth);
return 0;
-put:
- drm_dev_put(drm);
unregister_pix_clk:
clk_unregister(fsl_dev->pix_clk);
disable_clk:
@@ -349,9 +343,9 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
static int fsl_dcu_drm_remove(struct platform_device *pdev)
{
struct fsl_dcu_drm_device *fsl_dev = platform_get_drvdata(pdev);
+ struct drm_device *drm = &fsl_dev->base;
- drm_dev_unregister(fsl_dev->drm);
- drm_dev_put(fsl_dev->drm);
+ drm_dev_unregister(drm);
clk_disable_unprepare(fsl_dev->clk);
clk_unregister(fsl_dev->pix_clk);
di
[PATCH drm-misc-next v2 2/9] drm/fsl-dcu: replace drm->dev_private with drm_to_fsl_dcu_drm_dev()
Using drm_device->dev_private is deprecated. Since we've switched to
devm_drm_dev_alloc(), struct drm_device is now embedded in struct
malidp_drm, hence we can use container_of() to get the struct drm_device
instance instead.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 12 ++--
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 13 -
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 2 ++
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 8
4 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index a1b8ce70928a..e05311e2b0e0 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -24,7 +24,7 @@ static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc
*crtc,
struct drm_atomic_state *state)
{
struct drm_device *dev = crtc->dev;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
struct drm_pending_vblank_event *event = crtc->state->event;
regmap_write(fsl_dev->regmap,
@@ -48,7 +48,7 @@ static void fsl_dcu_drm_crtc_atomic_disable(struct drm_crtc
*crtc,
struct drm_crtc_state *old_crtc_state =
drm_atomic_get_old_crtc_state(state,
crtc);
struct drm_device *dev = crtc->dev;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
/* always disable planes on the CRTC */
drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, true);
@@ -67,7 +67,7 @@ static void fsl_dcu_drm_crtc_atomic_enable(struct drm_crtc
*crtc,
struct drm_atomic_state *state)
{
struct drm_device *dev = crtc->dev;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
clk_prepare_enable(fsl_dev->pix_clk);
regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
@@ -82,7 +82,7 @@ static void fsl_dcu_drm_crtc_atomic_enable(struct drm_crtc
*crtc,
static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
struct drm_connector *con = &fsl_dev->connector.base;
struct drm_display_mode *mode = &crtc->state->mode;
unsigned int pol = 0;
@@ -135,7 +135,7 @@ static const struct drm_crtc_helper_funcs
fsl_dcu_drm_crtc_helper_funcs = {
static int fsl_dcu_drm_crtc_enable_vblank(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
unsigned int value;
regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
@@ -148,7 +148,7 @@ static int fsl_dcu_drm_crtc_enable_vblank(struct drm_crtc
*crtc)
static void fsl_dcu_drm_crtc_disable_vblank(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
unsigned int value;
regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index a47b72995fcf..4139f674c5de 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -52,7 +52,7 @@ static const struct regmap_config fsl_dcu_regmap_config = {
static void fsl_dcu_irq_reset(struct drm_device *dev)
{
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
regmap_write(fsl_dev->regmap, DCU_INT_STATUS, ~0);
regmap_write(fsl_dev->regmap, DCU_INT_MASK, ~0);
@@ -61,7 +61,7 @@ static void fsl_dcu_irq_reset(struct drm_device *dev)
static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
{
struct drm_device *dev = arg;
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
unsigned int int_status;
int ret;
@@ -91,7 +91,7 @@ static int fsl_dcu_irq_install(struct drm_device *dev,
unsigned int irq)
static void fsl_dcu_irq_uninstall(struct drm_device *dev)
{
- struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+ struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
fsl_dcu_irq_reset(dev);
free_irq(fsl_dev->irq, dev);
@@ -99,7 +99,7 @@ static void fsl_dcu
[PATCH drm-misc-next v2 3/9] drm/fsl-dcu: crtc: use drmm_crtc_init_with_planes()
Use drmm_crtc_init_with_planes() instead of drm_crtc_init_with_planes()
to get rid of the explicit destroy hook in struct drm_plane_funcs.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index e05311e2b0e0..0b70624260fc 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -159,7 +159,6 @@ static void fsl_dcu_drm_crtc_disable_vblank(struct drm_crtc
*crtc)
static const struct drm_crtc_funcs fsl_dcu_drm_crtc_funcs = {
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
- .destroy = drm_crtc_cleanup,
.page_flip = drm_atomic_helper_page_flip,
.reset = drm_atomic_helper_crtc_reset,
.set_config = drm_atomic_helper_set_config,
@@ -180,8 +179,8 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device
*fsl_dev)
if (!primary)
return -ENOMEM;
- ret = drm_crtc_init_with_planes(drm, crtc, primary, NULL,
- &fsl_dcu_drm_crtc_funcs, NULL);
+ ret = drmm_crtc_init_with_planes(drm, crtc, primary, NULL,
+&fsl_dcu_drm_crtc_funcs, NULL);
if (ret) {
primary->funcs->destroy(primary);
return ret;
--
2.37.3
[PATCH drm-misc-next v2 4/9] drm/fsl-dcu: plane: use drm managed resources
Use drm managed resource allocation (drmm_universal_plane_alloc()) in
order to get rid of the explicit destroy hook in struct drm_plane_funcs.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 4 ++--
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 25 -
2 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 0b70624260fc..1dad90f701c8 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -176,8 +176,8 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device
*fsl_dev)
fsl_dcu_drm_init_planes(drm);
primary = fsl_dcu_drm_primary_create_plane(drm);
- if (!primary)
- return -ENOMEM;
+ if (IS_ERR(primary))
+ return PTR_ERR(primary);
ret = drmm_crtc_init_with_planes(drm, crtc, primary, NULL,
&fsl_dcu_drm_crtc_funcs, NULL);
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 91865956da02..23ff285da477 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -174,7 +174,6 @@ static const struct drm_plane_helper_funcs
fsl_dcu_drm_plane_helper_funcs = {
static const struct drm_plane_funcs fsl_dcu_drm_plane_funcs = {
.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
- .destroy = drm_plane_helper_destroy,
.disable_plane = drm_atomic_helper_disable_plane,
.reset = drm_atomic_helper_plane_reset,
.update_plane = drm_atomic_helper_update_plane,
@@ -206,24 +205,18 @@ void fsl_dcu_drm_init_planes(struct drm_device *dev)
struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
{
struct drm_plane *primary;
- int ret;
-
- primary = kzalloc(sizeof(*primary), GFP_KERNEL);
- if (!primary) {
- DRM_DEBUG_KMS("Failed to allocate primary plane\n");
- return NULL;
- }
/* possible_crtc's will be filled in later by crtc_init */
- ret = drm_universal_plane_init(dev, primary, 0,
- &fsl_dcu_drm_plane_funcs,
- fsl_dcu_drm_plane_formats,
- ARRAY_SIZE(fsl_dcu_drm_plane_formats),
- NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
- if (ret) {
- kfree(primary);
- primary = NULL;
+ primary = drmm_universal_plane_alloc(dev, struct drm_plane, dev, 0,
+&fsl_dcu_drm_plane_funcs,
+fsl_dcu_drm_plane_formats,
+
ARRAY_SIZE(fsl_dcu_drm_plane_formats),
+NULL, DRM_PLANE_TYPE_PRIMARY,
NULL);
+ if (IS_ERR(primary)) {
+ DRM_DEBUG_KMS("Failed to create primary plane\n");
+ return primary;
}
+
drm_plane_helper_add(primary, &fsl_dcu_drm_plane_helper_funcs);
return primary;
--
2.37.3
[PATCH drm-misc-next v2 5/9] drm/fsl-dcu: use drm_dev_unplug()
When the driver is unbound, there might still be users in userspace having an open fd and are calling into the driver. While this is fine for drm managed resources, it is not for resources bound to the device/driver lifecycle, e.g. clocks or MMIO mappings. To prevent use-after-free issues we need to protect those resources with drm_dev_enter() and drm_dev_exit(). This does only work if we indicate that the drm device was unplugged, hence use drm_dev_unplug() instead of drm_dev_unregister(). Protecting the particular resources with drm_dev_enter()/drm_dev_exit() is handled by subsequent patches. Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index 4139f674c5de..3ac57516c3fe 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -340,7 +340,7 @@ static int fsl_dcu_drm_remove(struct platform_device *pdev) struct fsl_dcu_drm_device *fsl_dev = platform_get_drvdata(pdev); struct drm_device *drm = &fsl_dev->base; - drm_dev_unregister(drm); + drm_dev_unplug(drm); clk_disable_unprepare(fsl_dev->clk); clk_unregister(fsl_dev->pix_clk); -- 2.37.3
[PATCH drm-misc-next v2 6/9] drm/fsl-dcu: plane: protect device resources after removal
(Hardware) resources which are bound to the driver and device lifecycle
must not be accessed after the device and driver are unbound.
However, the DRM device isn't freed as long as the last user didn't
close it, hence userspace can still call into the driver.
Therefore protect the critical sections which are accessing those
resources with drm_dev_enter() and drm_dev_exit().
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 24 +++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 23ff285da477..b1305f0af9d5 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -65,15 +66,21 @@ static void fsl_dcu_drm_plane_atomic_disable(struct
drm_plane *plane,
{
struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(plane->dev);
unsigned int value;
- int index;
+ int index, idx;
+
+ if (!drm_dev_enter(plane->dev, &idx))
+ return;
index = fsl_dcu_drm_plane_index(plane);
if (index < 0)
- return;
+ goto out;
regmap_read(fsl_dev->regmap, DCU_CTRLDESCLN(index, 4), &value);
value &= ~DCU_LAYER_EN;
regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(index, 4), value);
+
+out:
+ drm_dev_exit(idx);
}
static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
@@ -86,14 +93,17 @@ static void fsl_dcu_drm_plane_atomic_update(struct
drm_plane *plane,
struct drm_framebuffer *fb = plane->state->fb;
struct drm_gem_dma_object *gem;
unsigned int alpha = DCU_LAYER_AB_NONE, bpp;
- int index;
+ int index, idx;
- if (!fb)
+ if (!drm_dev_enter(plane->dev, &idx))
return;
+ if (!fb)
+ goto out;
+
index = fsl_dcu_drm_plane_index(plane);
if (index < 0)
- return;
+ goto out;
gem = drm_fb_dma_get_gem_obj(fb, 0);
@@ -126,7 +136,7 @@ static void fsl_dcu_drm_plane_atomic_update(struct
drm_plane *plane,
bpp = FSL_DCU_YUV422;
break;
default:
- return;
+ goto out;
}
regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(index, 1),
@@ -162,6 +172,8 @@ static void fsl_dcu_drm_plane_atomic_update(struct
drm_plane *plane,
DCU_LAYER_PRE_SKIP(0));
}
+out:
+ drm_dev_exit(idx);
return;
}
--
2.37.3
[PATCH drm-misc-next v2 8/9] drm/fsl-dcu: remove trailing return statements
Remove the trailing return statements at the end of void functions.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 1 -
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index c77df9b7893f..23687551c831 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -147,7 +147,6 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc
*crtc)
DCU_THRESHOLD_OUT_BUF_LOW(BUF_MIN_VAL));
drm_dev_exit(idx);
- return;
}
static const struct drm_crtc_helper_funcs fsl_dcu_drm_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index b1305f0af9d5..b95dca47de3e 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -174,7 +174,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct
drm_plane *plane,
out:
drm_dev_exit(idx);
- return;
}
static const struct drm_plane_helper_funcs fsl_dcu_drm_plane_helper_funcs = {
--
2.37.3
[PATCH drm-misc-next v2 9/9] drm/fsl-dcu: remove calls to drm_mode_config_cleanup()
drm_mode_config_init() simply calls drmm_mode_config_init(), hence cleanup is automatically handled through registering drm_mode_config_cleanup() with drmm_add_action_or_reset(). While at it, get rid of the deprecated drm_mode_config_init() and replace it with drmm_mode_config_init() directly. Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 4 +--- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 5 +++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c index 3ac57516c3fe..cb74ae663f25 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c @@ -128,10 +128,9 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags) } return 0; + done_irq: drm_kms_helper_poll_fini(dev); - - drm_mode_config_cleanup(dev); done_vblank: return ret; } @@ -141,7 +140,6 @@ static void fsl_dcu_unload(struct drm_device *dev) drm_atomic_helper_shutdown(dev); drm_kms_helper_poll_fini(dev); - drm_mode_config_cleanup(dev); fsl_dcu_irq_uninstall(dev); } diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c index c3d55c0aca9f..219ca539dedd 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c @@ -23,7 +23,9 @@ int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) struct drm_device *drm = &fsl_dev->base; int ret; - drm_mode_config_init(drm); + ret = drmm_mode_config_init(drm); + if (ret) + goto err; drm->mode_config.min_width = 0; drm->mode_config.min_height = 0; @@ -49,6 +51,5 @@ int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev) return 0; err: - drm_mode_config_cleanup(drm); return ret; } -- 2.37.3
[PATCH drm-misc-next v2 7/9] drm/fsl-dcu: crtc: protect device resources after removal
(Hardware) resources which are bound to the driver and device lifecycle
must not be accessed after the device and driver are unbound.
However, the DRM device isn't freed as long as the last user didn't
close it, hence userspace can still call into the driver.
Therefore protect the critical sections which are accessing those
resources with drm_dev_enter() and drm_dev_exit().
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 37 ++
1 file changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 1dad90f701c8..c77df9b7893f 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
@@ -26,6 +27,10 @@ static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc
*crtc,
struct drm_device *dev = crtc->dev;
struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
struct drm_pending_vblank_event *event = crtc->state->event;
+ int idx;
+
+ if (!drm_dev_enter(dev, &idx))
+ return;
regmap_write(fsl_dev->regmap,
DCU_UPDATE_MODE, DCU_UPDATE_MODE_READREG);
@@ -40,6 +45,8 @@ static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc
*crtc,
drm_crtc_send_vblank_event(crtc, event);
spin_unlock_irq(&crtc->dev->event_lock);
}
+
+ drm_dev_exit(idx);
}
static void fsl_dcu_drm_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -49,6 +56,10 @@ static void fsl_dcu_drm_crtc_atomic_disable(struct drm_crtc
*crtc,
crtc);
struct drm_device *dev = crtc->dev;
struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
+ int idx;
+
+ if (!drm_dev_enter(dev, &idx))
+ return;
/* always disable planes on the CRTC */
drm_atomic_helper_disable_planes_on_crtc(old_crtc_state, true);
@@ -61,6 +72,8 @@ static void fsl_dcu_drm_crtc_atomic_disable(struct drm_crtc
*crtc,
regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
DCU_UPDATE_MODE_READREG);
clk_disable_unprepare(fsl_dev->pix_clk);
+
+ drm_dev_exit(idx);
}
static void fsl_dcu_drm_crtc_atomic_enable(struct drm_crtc *crtc,
@@ -68,6 +81,10 @@ static void fsl_dcu_drm_crtc_atomic_enable(struct drm_crtc
*crtc,
{
struct drm_device *dev = crtc->dev;
struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
+ int idx;
+
+ if (!drm_dev_enter(dev, &idx))
+ return;
clk_prepare_enable(fsl_dev->pix_clk);
regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
@@ -77,6 +94,8 @@ static void fsl_dcu_drm_crtc_atomic_enable(struct drm_crtc
*crtc,
DCU_UPDATE_MODE_READREG);
drm_crtc_vblank_on(crtc);
+
+ drm_dev_exit(idx);
}
static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
@@ -87,6 +106,10 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc
*crtc)
struct drm_display_mode *mode = &crtc->state->mode;
unsigned int pol = 0;
struct videomode vm;
+ int idx;
+
+ if (!drm_dev_enter(dev, &idx))
+ return;
clk_set_rate(fsl_dev->pix_clk, mode->clock * 1000);
@@ -122,6 +145,8 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc
*crtc)
DCU_THRESHOLD_LS_BF_VS(BF_VS_VAL) |
DCU_THRESHOLD_OUT_BUF_HIGH(BUF_MAX_VAL) |
DCU_THRESHOLD_OUT_BUF_LOW(BUF_MIN_VAL));
+
+ drm_dev_exit(idx);
return;
}
@@ -137,11 +162,17 @@ static int fsl_dcu_drm_crtc_enable_vblank(struct drm_crtc
*crtc)
struct drm_device *dev = crtc->dev;
struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
unsigned int value;
+ int idx;
+
+ if (!drm_dev_enter(dev, &idx))
+ return -ENODEV;
regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
value &= ~DCU_INT_MASK_VBLANK;
regmap_write(fsl_dev->regmap, DCU_INT_MASK, value);
+ drm_dev_exit(idx);
+
return 0;
}
@@ -150,10 +181,16 @@ static void fsl_dcu_drm_crtc_disable_vblank(struct
drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct fsl_dcu_drm_device *fsl_dev = drm_to_fsl_dcu_drm_dev(dev);
unsigned int value;
+ int idx;
+
+ if (!drm_dev_enter(dev, &idx))
+ return;
regmap_read(fsl_dev->regmap, DCU_INT_MASK, &value);
value |= DCU_INT_MASK_VBLANK;
regmap_write(fsl_dev->regmap, DCU_INT_MASK, value);
+
+ drm_dev_exit(idx);
}
static const struct drm_crtc_funcs fsl_dcu_drm_crtc_funcs = {
--
2.37.3
Re: [PATCH v3 2/2] drm/msm/dsi: Add phy configuration for QCM2290
On 2022-09-24 15:19:00, Dmitry Baryshkov wrote:
> From: Loic Poulain
>
> The QCM2290 SoC a the 14nm (V2.0) single DSI phy. The platform is not
> fully compatible with the standard 14nm PHY, so it requires a separate
> compatible and config entry.
>
> Signed-off-by: Loic Poulain
> [DB: rebased and updated commit msg]
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 2 ++
> drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 1 +
> drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 17 +
> 3 files changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> index 7fc0975cb869..ee6051367679 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> @@ -549,6 +549,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
> #ifdef CONFIG_DRM_MSM_DSI_14NM_PHY
> { .compatible = "qcom,dsi-phy-14nm",
> .data = &dsi_phy_14nm_cfgs },
> + { .compatible = "qcom,dsi-phy-14nm-2290",
> + .data = &dsi_phy_14nm_2290_cfgs },
> { .compatible = "qcom,dsi-phy-14nm-660",
> .data = &dsi_phy_14nm_660_cfgs },
> { .compatible = "qcom,dsi-phy-14nm-8953",
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> index 60a99c6525b2..1096afedd616 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> @@ -50,6 +50,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_20nm_cfgs;
> extern const struct msm_dsi_phy_cfg dsi_phy_28nm_8960_cfgs;
> extern const struct msm_dsi_phy_cfg dsi_phy_14nm_cfgs;
> extern const struct msm_dsi_phy_cfg dsi_phy_14nm_660_cfgs;
> +extern const struct msm_dsi_phy_cfg dsi_phy_14nm_2290_cfgs;
following alphabetical sorting (same as the other locations in this
series), this should be above 660?
> extern const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs;
> extern const struct msm_dsi_phy_cfg dsi_phy_10nm_cfgs;
> extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> index 0f8f4ca46429..9f488adea7f5 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> @@ -1081,3 +1081,20 @@ const struct msm_dsi_phy_cfg dsi_phy_14nm_8953_cfgs = {
> .io_start = { 0x1a94400, 0x1a96400 },
> .num_dsi_phy = 2,
> };
> +
> +const struct msm_dsi_phy_cfg dsi_phy_14nm_2290_cfgs = {
> + .has_phy_lane = true,
> + .regulator_data = dsi_phy_14nm_17mA_regulators,
> + .num_regulators = ARRAY_SIZE(dsi_phy_14nm_17mA_regulators),
> + .ops = {
> + .enable = dsi_14nm_phy_enable,
> + .disable = dsi_14nm_phy_disable,
> + .pll_init = dsi_pll_14nm_init,
> + .save_pll_state = dsi_14nm_pll_save_state,
> + .restore_pll_state = dsi_14nm_pll_restore_state,
> + },
> + .min_pll_rate = VCO_MIN_RATE,
> + .max_pll_rate = VCO_MAX_RATE,
> + .io_start = { 0x5e94400 },
For sm6125 we also need this exact io_start (and a single PHY), do you
think it makes sense to add a compatible that reuses the same struct (I
can do that in a folloup patch) and/or generalize this struct (name)?
However, our regulator setup appears to be different. I recall not
finding any `vcca` supply in my downstream sources, and had this in my
notes for a similar dsi_phy_14nm.c patch:
sm6125 uses an RPM regulator
https://github.com/sonyxperiadev/kernel/blob/f956fbd9a234033bd18234d456a2c32c126b38f3/arch/arm64/boot/dts/qcom/trinket-sde.dtsi#L388
- Marijn
> + .num_dsi_phy = 1,
> +};
> --
> 2.35.1
>
Re: [PATCH v2 01/16] slab: Remove __malloc attribute from realloc functions
On Fri, Sep 23, 2022 at 01:28:07PM -0700, Kees Cook wrote:
> The __malloc attribute should not be applied to "realloc" functions, as
> the returned pointer may alias the storage of the prior pointer. Instead
> of splitting __malloc from __alloc_size, which would be a huge amount of
> churn, just create __realloc_size for the few cases where it is needed.
>
> Additionally removes the conditional test for __alloc_size__, which is
> always defined now.
>
> Cc: Christoph Lameter
> Cc: Pekka Enberg
> Cc: David Rientjes
> Cc: Joonsoo Kim
> Cc: Andrew Morton
> Cc: Vlastimil Babka
> Cc: Roman Gushchin
> Cc: Hyeonggon Yoo <42.hye...@gmail.com>
> Cc: Marco Elver
> Cc: linux...@kvack.org
> Signed-off-by: Kees Cook
> ---
> include/linux/compiler_types.h | 13 +
> include/linux/slab.h | 12 ++--
> mm/slab_common.c | 4 ++--
> 3 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 4f2a819fd60a..f141a6f6b9f6 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -271,15 +271,12 @@ struct ftrace_likely_data {
>
> /*
> * Any place that could be marked with the "alloc_size" attribute is also
> - * a place to be marked with the "malloc" attribute. Do this as part of the
> - * __alloc_size macro to avoid redundant attributes and to avoid missing a
> - * __malloc marking.
> + * a place to be marked with the "malloc" attribute, except those that may
> + * be performing a _reallocation_, as that may alias the existing pointer.
> + * For these, use __realloc_size().
> */
> -#ifdef __alloc_size__
> -# define __alloc_size(x, ...)__alloc_size__(x, ## __VA_ARGS__)
> __malloc
> -#else
> -# define __alloc_size(x, ...)__malloc
> -#endif
> +#define __alloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__) __malloc
> +#define __realloc_size(x, ...) __alloc_size__(x, ## __VA_ARGS__)
>
> #ifndef asm_volatile_goto
> #define asm_volatile_goto(x...) asm goto(x)
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 0fefdf528e0d..41bd036e7551 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -184,7 +184,7 @@ int kmem_cache_shrink(struct kmem_cache *s);
> /*
> * Common kmalloc functions provided by all allocators
> */
> -void * __must_check krealloc(const void *objp, size_t new_size, gfp_t flags)
> __alloc_size(2);
> +void * __must_check krealloc(const void *objp, size_t new_size, gfp_t flags)
> __realloc_size(2);
> void kfree(const void *objp);
> void kfree_sensitive(const void *objp);
> size_t __ksize(const void *objp);
> @@ -647,10 +647,10 @@ static inline __alloc_size(1, 2) void
> *kmalloc_array(size_t n, size_t size, gfp_
> * @new_size: new size of a single member of the array
> * @flags: the type of memory to allocate (see kmalloc)
> */
> -static inline __alloc_size(2, 3) void * __must_check krealloc_array(void *p,
> - size_t
> new_n,
> - size_t
> new_size,
> - gfp_t flags)
> +static inline __realloc_size(2, 3) void * __must_check krealloc_array(void
> *p,
> + size_t
> new_n,
> + size_t
> new_size,
> + gfp_t
> flags)
> {
> size_t bytes;
>
> @@ -774,7 +774,7 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n,
> size_t size, gfp_t fla
> }
>
> extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t
> flags)
> - __alloc_size(3);
> + __realloc_size(3);
> extern void kvfree(const void *addr);
> extern void kvfree_sensitive(const void *addr, size_t len);
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 17996649cfe3..457671ace7eb 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1134,8 +1134,8 @@ module_init(slab_proc_init);
>
> #endif /* CONFIG_SLAB || CONFIG_SLUB_DEBUG */
>
> -static __always_inline void *__do_krealloc(const void *p, size_t new_size,
> -gfp_t flags)
> +static __always_inline __realloc_size(2) void *
> +__do_krealloc(const void *p, size_t new_size, gfp_t flags)
> {
> void *ret;
> size_t ks;
> --
> 2.34.1
>
This is now squashed with later one. (so undefined __alloc_size__ issues are
fixed)
for the latest version of this patch:
Looks good to me,
Acked-by: Hyeonggon Yoo <42.hye...@gmail.com>
--
Thanks,
Hyeonggon
Re: [PATCH v2 02/16] slab: Introduce kmalloc_size_roundup()
On Fri, Sep 23, 2022 at 01:28:08PM -0700, Kees Cook wrote: > In the effort to help the compiler reason about buffer sizes, the > __alloc_size attribute was added to allocators. This improves the scope > of the compiler's ability to apply CONFIG_UBSAN_BOUNDS and (in the near > future) CONFIG_FORTIFY_SOURCE. For most allocations, this works well, > as the vast majority of callers are not expecting to use more memory > than what they asked for. > > There is, however, one common exception to this: anticipatory resizing > of kmalloc allocations. These cases all use ksize() to determine the > actual bucket size of a given allocation (e.g. 128 when 126 was asked > for). This comes in two styles in the kernel: > > 1) An allocation has been determined to be too small, and needs to be >resized. Instead of the caller choosing its own next best size, it >wants to minimize the number of calls to krealloc(), so it just uses >ksize() plus some additional bytes, forcing the realloc into the next >bucket size, from which it can learn how large it is now. For example: > > data = krealloc(data, ksize(data) + 1, gfp); > data_len = ksize(data); > > 2) The minimum size of an allocation is calculated, but since it may >grow in the future, just use all the space available in the chosen >bucket immediately, to avoid needing to reallocate later. A good >example of this is skbuff's allocators: > > data = kmalloc_reserve(size, gfp_mask, node, &pfmemalloc); > ... > /* kmalloc(size) might give us more room than requested. >* Put skb_shared_info exactly at the end of allocated zone, >* to allow max possible filling before reallocation. >*/ > osize = ksize(data); > size = SKB_WITH_OVERHEAD(osize); > > In both cases, the "how much was actually allocated?" question is answered > _after_ the allocation, where the compiler hinting is not in an easy place > to make the association any more. This mismatch between the compiler's > view of the buffer length and the code's intention about how much it is > going to actually use has already caused problems[1]. It is possible to > fix this by reordering the use of the "actual size" information. > > We can serve the needs of users of ksize() and still have accurate buffer > length hinting for the compiler by doing the bucket size calculation > _before_ the allocation. Code can instead ask "how large an allocation > would I get for a given size?". > > Introduce kmalloc_size_roundup(), to serve this function so we can start > replacing the "anticipatory resizing" uses of ksize(). > > [1] https://github.com/ClangBuiltLinux/linux/issues/1599 > https://github.com/KSPP/linux/issues/183 > > Cc: Vlastimil Babka > Cc: Christoph Lameter > Cc: Pekka Enberg > Cc: David Rientjes > Cc: Joonsoo Kim > Cc: Andrew Morton > Cc: linux...@kvack.org > Signed-off-by: Kees Cook > --- > include/linux/slab.h | 31 +++ > mm/slab.c| 9 ++--- > mm/slab_common.c | 20 > 3 files changed, 57 insertions(+), 3 deletions(-) > > diff --git a/include/linux/slab.h b/include/linux/slab.h > index 41bd036e7551..727640173568 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -188,7 +188,21 @@ void * __must_check krealloc(const void *objp, size_t > new_size, gfp_t flags) __r > void kfree(const void *objp); > void kfree_sensitive(const void *objp); > size_t __ksize(const void *objp); > + > +/** > + * ksize - Report actual allocation size of associated object > + * > + * @objp: Pointer returned from a prior kmalloc()-family allocation. > + * > + * This should not be used for writing beyond the originally requested > + * allocation size. Either use krealloc() or round up the allocation size > + * with kmalloc_size_roundup() prior to allocation. If this is used to > + * access beyond the originally requested allocation size, UBSAN_BOUNDS > + * and/or FORTIFY_SOURCE may trip, since they only know about the > + * originally allocated size via the __alloc_size attribute. > + */ > size_t ksize(const void *objp); > + With this now we have two conflicting kernel-doc comments about ksize in mm/slab_common.c and include/linux/slab.h. > #ifdef CONFIG_PRINTK > bool kmem_valid_obj(void *object); > void kmem_dump_obj(void *object); > @@ -779,6 +793,23 @@ extern void kvfree(const void *addr); > extern void kvfree_sensitive(const void *addr, size_t len); > > unsigned int kmem_cache_size(struct kmem_cache *s); > + > +/** > + * kmalloc_size_roundup - Report allocation bucket size for the given size > + * > + * @size: Number of bytes to round up from. > + * > + * This returns the number of bytes that would be available in a kmalloc() > + * allocation of @size bytes. For example, a 126 byte request would be > + * rounded up to the next sized kmalloc bucket, 128 bytes. (This is strictly > + * for the general-purpose kmalloc()-based allocations, an
[PATCH drm-misc-next v2 0/9] drm/arm/malidp: use drm managed resources
Hi,
This patch series converts the driver to use drm managed resources to prevent
potential use-after-free issues on driver unbind/rebind and to get rid of the
usage of deprecated APIs.
Changes in v2:
- While protecting critical sections with drm_dev_{enter,exit} I forgot to
handle alternate return paths within the read-side critical sections, hence
fix them.
- Add a patch to remove explicit calls to drm_mode_config_cleanup() and switch
to drmm_mode_config_init() explicitly.
Danilo Krummrich (9):
drm/arm/malidp: use drmm_* to allocate driver structures
drm/arm/malidp: replace drm->dev_private with drm_to_malidp()
drm/arm/malidp: crtc: use drmm_crtc_init_with_planes()
drm/arm/malidp: plane: use drm managed resources
drm/arm/malidp: use drm_dev_unplug()
drm/arm/malidp: plane: protect device resources after removal
drm/arm/malidp: crtc: protect device resources after removal
drm/arm/malidp: drv: protect device resources after removal
drm/arm/malidp: remove calls to drm_mode_config_cleanup()
drivers/gpu/drm/arm/malidp_crtc.c | 68 +++--
drivers/gpu/drm/arm/malidp_drv.c| 78 -
drivers/gpu/drm/arm/malidp_drv.h| 2 +
drivers/gpu/drm/arm/malidp_hw.c | 10 ++--
drivers/gpu/drm/arm/malidp_mw.c | 6 +--
drivers/gpu/drm/arm/malidp_planes.c | 45 -
6 files changed, 117 insertions(+), 92 deletions(-)
base-commit: 08fb97de03aa2205c6791301bd83a095abc1949c
--
2.37.3
[PATCH drm-misc-next v2 1/9] drm/arm/malidp: use drmm_* to allocate driver structures
Use drm managed resources to allocate driver structures and get rid of
the deprecated drm_dev_alloc() call and replace it with
devm_drm_dev_alloc().
This also serves as preparation to get rid of drm_device->dev_private
and to fix use-after-free issues on driver unload.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/arm/malidp_drv.c | 20 +++-
drivers/gpu/drm/arm/malidp_drv.h | 1 +
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 1d0b0c54ccc7..41c80e905991 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -716,11 +717,13 @@ static int malidp_bind(struct device *dev)
int ret = 0, i;
u32 version, out_depth = 0;
- malidp = devm_kzalloc(dev, sizeof(*malidp), GFP_KERNEL);
- if (!malidp)
- return -ENOMEM;
+ malidp = devm_drm_dev_alloc(dev, &malidp_driver, typeof(*malidp), base);
+ if (IS_ERR(malidp))
+ return PTR_ERR(malidp);
+
+ drm = &malidp->base;
- hwdev = devm_kzalloc(dev, sizeof(*hwdev), GFP_KERNEL);
+ hwdev = drmm_kzalloc(drm, sizeof(*hwdev), GFP_KERNEL);
if (!hwdev)
return -ENOMEM;
@@ -753,12 +756,6 @@ static int malidp_bind(struct device *dev)
if (ret && ret != -ENODEV)
return ret;
- drm = drm_dev_alloc(&malidp_driver, dev);
- if (IS_ERR(drm)) {
- ret = PTR_ERR(drm);
- goto alloc_fail;
- }
-
drm->dev_private = malidp;
dev_set_drvdata(dev, drm);
@@ -887,8 +884,6 @@ static int malidp_bind(struct device *dev)
malidp_runtime_pm_suspend(dev);
drm->dev_private = NULL;
dev_set_drvdata(dev, NULL);
- drm_dev_put(drm);
-alloc_fail:
of_reserved_mem_device_release(dev);
return ret;
@@ -917,7 +912,6 @@ static void malidp_unbind(struct device *dev)
malidp_runtime_pm_suspend(dev);
drm->dev_private = NULL;
dev_set_drvdata(dev, NULL);
- drm_dev_put(drm);
of_reserved_mem_device_release(dev);
}
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index cdfddfabf2d1..00be369b28f1 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -29,6 +29,7 @@ struct malidp_error_stats {
};
struct malidp_drm {
+ struct drm_device base;
struct malidp_hw_device *dev;
struct drm_crtc crtc;
struct drm_writeback_connector mw_connector;
--
2.37.3
[PATCH drm-misc-next v2 2/9] drm/arm/malidp: replace drm->dev_private with drm_to_malidp()
Using drm_device->dev_private is deprecated. Since we've switched to
devm_drm_dev_alloc(), struct drm_device is now embedded in struct
malidp_drm, hence we can use container_of() to get the struct drm_device
instance instead.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/arm/malidp_crtc.c | 2 +-
drivers/gpu/drm/arm/malidp_drv.c| 29 +
drivers/gpu/drm/arm/malidp_drv.h| 1 +
drivers/gpu/drm/arm/malidp_hw.c | 10 +-
drivers/gpu/drm/arm/malidp_mw.c | 6 +++---
drivers/gpu/drm/arm/malidp_planes.c | 4 ++--
6 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
b/drivers/gpu/drm/arm/malidp_crtc.c
index 962730772b2f..34ad7e1cd2b8 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -526,7 +526,7 @@ static const struct drm_crtc_funcs malidp_crtc_funcs = {
int malidp_crtc_init(struct drm_device *drm)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct drm_plane *primary = NULL, *plane;
int ret;
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 41c80e905991..678c5b0d8014 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -169,7 +169,7 @@ static void malidp_atomic_commit_se_config(struct drm_crtc
*crtc,
*/
static int malidp_set_and_wait_config_valid(struct drm_device *drm)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
int ret;
@@ -190,7 +190,7 @@ static int malidp_set_and_wait_config_valid(struct
drm_device *drm)
static void malidp_atomic_commit_hw_done(struct drm_atomic_state *state)
{
struct drm_device *drm = state->dev;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
int loop = 5;
malidp->event = malidp->crtc.state->event;
@@ -231,7 +231,7 @@ static void malidp_atomic_commit_hw_done(struct
drm_atomic_state *state)
static void malidp_atomic_commit_tail(struct drm_atomic_state *state)
{
struct drm_device *drm = state->dev;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state;
int i;
@@ -393,7 +393,7 @@ static const struct drm_mode_config_funcs
malidp_mode_config_funcs = {
static int malidp_init(struct drm_device *drm)
{
int ret;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
drm_mode_config_init(drm);
@@ -429,7 +429,7 @@ static int malidp_irq_init(struct platform_device *pdev)
{
int irq_de, irq_se, ret = 0;
struct drm_device *drm = dev_get_drvdata(&pdev->dev);
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
/* fetch the interrupts from DT */
@@ -463,7 +463,7 @@ static int malidp_dumb_create(struct drm_file *file_priv,
struct drm_device *drm,
struct drm_mode_create_dumb *args)
{
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
/* allocate for the worst case scenario, i.e. rotated buffers */
u8 alignment = malidp_hw_get_pitch_align(malidp->dev, 1);
@@ -509,7 +509,7 @@ static void malidp_error_stats_dump(const char *prefix,
static int malidp_show_stats(struct seq_file *m, void *arg)
{
struct drm_device *drm = m->private;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
unsigned long irqflags;
struct malidp_error_stats de_errors, se_errors;
@@ -532,7 +532,7 @@ static ssize_t malidp_debugfs_write(struct file *file,
const char __user *ubuf,
{
struct seq_file *m = file->private_data;
struct drm_device *drm = m->private;
- struct malidp_drm *malidp = drm->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(drm);
unsigned long irqflags;
spin_lock_irqsave(&malidp->errors_lock, irqflags);
@@ -553,7 +553,7 @@ static const struct file_operations malidp_debugfs_fops = {
static void malidp_debugfs_init(struct drm_minor *minor)
{
- struct malidp_drm *malidp = minor->dev->dev_private;
+ struct malidp_drm *malidp = drm_to_malidp(minor->dev);
malidp_error_stats_init(&malidp->de_errors);
malidp_error_stats_init(&malidp->se_errors);
@@ -653,7 +653,7 @@ static ssize_t core_id_show(struct device *dev, struct
device_attribute *attr,
char *buf)
{
[PATCH drm-misc-next v2 3/9] drm/arm/malidp: crtc: use drmm_crtc_init_with_planes()
Use drmm_crtc_init_with_planes() instead of drm_crtc_init_with_planes()
to get rid of the explicit destroy hook in struct drm_plane_funcs.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/arm/malidp_crtc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
b/drivers/gpu/drm/arm/malidp_crtc.c
index 34ad7e1cd2b8..dc01c43f6193 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -514,7 +514,6 @@ static void malidp_crtc_disable_vblank(struct drm_crtc
*crtc)
}
static const struct drm_crtc_funcs malidp_crtc_funcs = {
- .destroy = drm_crtc_cleanup,
.set_config = drm_atomic_helper_set_config,
.page_flip = drm_atomic_helper_page_flip,
.reset = malidp_crtc_reset,
@@ -548,8 +547,8 @@ int malidp_crtc_init(struct drm_device *drm)
return -EINVAL;
}
- ret = drm_crtc_init_with_planes(drm, &malidp->crtc, primary, NULL,
- &malidp_crtc_funcs, NULL);
+ ret = drmm_crtc_init_with_planes(drm, &malidp->crtc, primary, NULL,
+&malidp_crtc_funcs, NULL);
if (ret)
return ret;
--
2.37.3
[PATCH drm-misc-next v2 4/9] drm/arm/malidp: plane: use drm managed resources
Use drm managed resource allocation (drmm_universal_plane_alloc()) in
order to get rid of the explicit destroy hook in struct drm_plane_funcs.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/arm/malidp_planes.c | 28 +++-
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index 815d9199752f..34547edf1ee3 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -68,14 +68,6 @@
/* readahead for partial-frame prefetch */
#define MALIDP_MMU_PREFETCH_READAHEAD 8
-static void malidp_de_plane_destroy(struct drm_plane *plane)
-{
- struct malidp_plane *mp = to_malidp_plane(plane);
-
- drm_plane_cleanup(plane);
- kfree(mp);
-}
-
/*
* Replicate what the default ->reset hook does: free the state pointer and
* allocate a new empty object. We just need enough space to store
@@ -260,7 +252,6 @@ static bool malidp_format_mod_supported_per_plane(struct
drm_plane *plane,
static const struct drm_plane_funcs malidp_de_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
- .destroy = malidp_de_plane_destroy,
.reset = malidp_plane_reset,
.atomic_duplicate_state = malidp_duplicate_plane_state,
.atomic_destroy_state = malidp_destroy_plane_state,
@@ -972,12 +963,6 @@ int malidp_de_planes_init(struct drm_device *drm)
for (i = 0; i < map->n_layers; i++) {
u8 id = map->layers[i].id;
- plane = kzalloc(sizeof(*plane), GFP_KERNEL);
- if (!plane) {
- ret = -ENOMEM;
- goto cleanup;
- }
-
/* build the list of DRM supported formats based on the map */
for (n = 0, j = 0; j < map->n_pixel_formats; j++) {
if ((map->pixel_formats[j].layer & id) == id)
@@ -990,13 +975,14 @@ int malidp_de_planes_init(struct drm_device *drm)
/*
* All the layers except smart layer supports AFBC modifiers.
*/
- ret = drm_universal_plane_init(drm, &plane->base, crtcs,
- &malidp_de_plane_funcs, formats, n,
- (id == DE_SMART) ? linear_only_modifiers :
modifiers,
- plane_type, NULL);
-
- if (ret < 0)
+ plane = drmm_universal_plane_alloc(drm, struct malidp_plane,
base,
+ crtcs,
&malidp_de_plane_funcs, formats, n,
+ (id == DE_SMART) ?
linear_only_modifiers :
+ modifiers, plane_type, NULL);
+ if (IS_ERR(plane)) {
+ ret = PTR_ERR(plane);
goto cleanup;
+ }
drm_plane_helper_add(&plane->base,
&malidp_de_plane_helper_funcs);
--
2.37.3
[PATCH drm-misc-next v2 5/9] drm/arm/malidp: use drm_dev_unplug()
When the driver is unbound, there might still be users in userspace having an open fd and are calling into the driver. While this is fine for drm managed resources, it is not for resources bound to the device/driver lifecycle, e.g. clocks or MMIO mappings. To prevent use-after-free issues we need to protect those resources with drm_dev_enter() and drm_dev_exit(). This does only work if we indicate that the drm device was unplugged, hence use drm_dev_unplug() instead of drm_dev_unregister(). Protecting the particular resources with drm_dev_enter()/drm_dev_exit() is handled by subsequent patches. Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/arm/malidp_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index 678c5b0d8014..aedd30f5f451 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -893,7 +893,7 @@ static void malidp_unbind(struct device *dev) struct malidp_drm *malidp = drm_to_malidp(drm); struct malidp_hw_device *hwdev = malidp->dev; - drm_dev_unregister(drm); + drm_dev_unplug(drm); drm_kms_helper_poll_fini(drm); pm_runtime_get_sync(dev); drm_atomic_helper_shutdown(drm); -- 2.37.3
[PATCH drm-misc-next v2 6/9] drm/arm/malidp: plane: protect device resources after removal
(Hardware) resources which are bound to the driver and device lifecycle
must not be accessed after the device and driver are unbound.
However, the DRM device isn't freed as long as the last user didn't
close it, hence userspace can still call into the driver.
Therefore protect the critical sections which are accessing those
resources with drm_dev_enter() and drm_dev_exit().
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/arm/malidp_planes.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index 34547edf1ee3..d2ea60549454 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -790,9 +790,12 @@ static void malidp_de_plane_update(struct drm_plane *plane,
u16 pixel_alpha = new_state->pixel_blend_mode;
u8 plane_alpha = new_state->alpha >> 8;
u32 src_w, src_h, dest_w, dest_h, val;
- int i;
+ int i, idx;
struct drm_framebuffer *fb = plane->state->fb;
+ if (!drm_dev_enter(plane->dev, &idx))
+ return;
+
mp = to_malidp_plane(plane);
/*
@@ -897,16 +900,24 @@ static void malidp_de_plane_update(struct drm_plane
*plane,
malidp_hw_write(mp->hwdev, val,
mp->layer->base + MALIDP_LAYER_CONTROL);
+
+ drm_dev_exit(idx);
}
static void malidp_de_plane_disable(struct drm_plane *plane,
struct drm_atomic_state *state)
{
struct malidp_plane *mp = to_malidp_plane(plane);
+ int idx;
+
+ if (!drm_dev_enter(plane->dev, &idx))
+ return;
malidp_hw_clearbits(mp->hwdev,
LAYER_ENABLE | LAYER_FLOWCFG(LAYER_FLOWCFG_MASK),
mp->layer->base + MALIDP_LAYER_CONTROL);
+
+ drm_dev_exit(idx);
}
static const struct drm_plane_helper_funcs malidp_de_plane_helper_funcs = {
--
2.37.3
[PATCH drm-misc-next v2 7/9] drm/arm/malidp: crtc: protect device resources after removal
(Hardware) resources which are bound to the driver and device lifecycle
must not be accessed after the device and driver are unbound.
However, the DRM device isn't freed as long as the last user didn't
close it, hence userspace can still call into the driver.
Therefore protect the critical sections which are accessing those
resources with drm_dev_enter() and drm_dev_exit().
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/arm/malidp_crtc.c | 61 +--
1 file changed, 50 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
b/drivers/gpu/drm/arm/malidp_crtc.c
index dc01c43f6193..e11cda5fdeb7 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -27,6 +28,8 @@ static enum drm_mode_status malidp_crtc_mode_valid(struct
drm_crtc *crtc,
{
struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
struct malidp_hw_device *hwdev = malidp->dev;
+ enum drm_mode_status status = MODE_OK;
+ int idx;
/*
* check that the hardware can drive the required clock rate,
@@ -34,15 +37,21 @@ static enum drm_mode_status malidp_crtc_mode_valid(struct
drm_crtc *crtc,
*/
long rate, req_rate = mode->crtc_clock * 1000;
+ if (!drm_dev_enter(&malidp->base, &idx))
+ return MODE_NOCLOCK;
+
if (req_rate) {
rate = clk_round_rate(hwdev->pxlclk, req_rate);
if (rate != req_rate) {
DRM_DEBUG_DRIVER("pxlclk doesn't support %ld Hz\n",
req_rate);
- return MODE_NOCLOCK;
+ status = MODE_NOCLOCK;
+ goto out;
}
}
+out:
+ drm_dev_exit(idx);
return MODE_OK;
}
@@ -52,11 +61,15 @@ static void malidp_crtc_atomic_enable(struct drm_crtc *crtc,
struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
struct malidp_hw_device *hwdev = malidp->dev;
struct videomode vm;
- int err = pm_runtime_get_sync(crtc->dev->dev);
+ int err, idx;
+
+ if (!drm_dev_enter(&malidp->base, &idx))
+ return;
+ err = pm_runtime_get_sync(crtc->dev->dev);
if (err < 0) {
DRM_DEBUG_DRIVER("Failed to enable runtime power management:
%d\n", err);
- return;
+ goto out;
}
drm_display_mode_to_videomode(&crtc->state->adjusted_mode, &vm);
@@ -68,6 +81,9 @@ static void malidp_crtc_atomic_enable(struct drm_crtc *crtc,
hwdev->hw->modeset(hwdev, &vm);
hwdev->hw->leave_config_mode(hwdev);
drm_crtc_vblank_on(crtc);
+
+out:
+ drm_dev_exit(idx);
}
static void malidp_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -77,7 +93,10 @@ static void malidp_crtc_atomic_disable(struct drm_crtc *crtc,
crtc);
struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
struct malidp_hw_device *hwdev = malidp->dev;
- int err;
+ int err, idx;
+
+ if (!drm_dev_enter(&malidp->base, &idx))
+ return;
/* always disable planes on the CRTC that is being turned off */
drm_atomic_helper_disable_planes_on_crtc(old_state, false);
@@ -91,6 +110,8 @@ static void malidp_crtc_atomic_disable(struct drm_crtc *crtc,
if (err < 0) {
DRM_DEBUG_DRIVER("Failed to disable runtime power management:
%d\n", err);
}
+
+ drm_dev_exit(idx);
}
static const struct gamma_curve_segment {
@@ -260,7 +281,10 @@ static int malidp_crtc_atomic_check_scaling(struct
drm_crtc *crtc,
u32 h_upscale_factor = 0; /* U16.16 */
u32 v_upscale_factor = 0; /* U16.16 */
u8 scaling = cs->scaled_planes_mask;
- int ret;
+ int idx, ret;
+
+ if (!drm_dev_enter(&malidp->base, &idx))
+ return -ENODEV;
if (!scaling) {
s->scale_enable = false;
@@ -268,8 +292,10 @@ static int malidp_crtc_atomic_check_scaling(struct
drm_crtc *crtc,
}
/* The scaling engine can only handle one plane at a time. */
- if (scaling & (scaling - 1))
- return -EINVAL;
+ if (scaling & (scaling - 1)) {
+ ret = -EINVAL;
+ goto out;
+ }
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
struct malidp_plane *mp = to_malidp_plane(plane);
@@ -331,10 +357,10 @@ static int malidp_crtc_atomic_check_scaling(struct
drm_crtc *crtc,
mclk_calc:
drm_display_mode_to_videomode(&state->adjusted_mode, &vm);
- ret = hwdev->hw->se_calc_mclk(hwdev, s, &vm);
- if (ret < 0)
- return -EINVAL;
- return 0;
+ ret = hwdev->hw->se_calc_mclk(hwdev, s, &vm) < 0 ? -EINVA
[PATCH drm-misc-next v2 9/9] drm/arm/malidp: remove calls to drm_mode_config_cleanup()
drm_mode_config_init() simply calls drmm_mode_config_init(), hence
cleanup is automatically handled through registering
drm_mode_config_cleanup() with drmm_add_action_or_reset().
While at it, get rid of the deprecated drm_mode_config_init() and
replace it with drmm_mode_config_init() directly.
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/arm/malidp_drv.c | 20 ++--
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 8bb8e8d14461..ef6a9fc1c864 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -401,7 +401,9 @@ static int malidp_init(struct drm_device *drm)
struct malidp_drm *malidp = drm_to_malidp(drm);
struct malidp_hw_device *hwdev = malidp->dev;
- drm_mode_config_init(drm);
+ ret = drmm_mode_config_init(drm);
+ if (ret)
+ goto out;
drm->mode_config.min_width = hwdev->min_line_size;
drm->mode_config.min_height = hwdev->min_line_size;
@@ -412,24 +414,16 @@ static int malidp_init(struct drm_device *drm)
ret = malidp_crtc_init(drm);
if (ret)
- goto crtc_fail;
+ goto out;
ret = malidp_mw_connector_init(drm);
if (ret)
- goto crtc_fail;
-
- return 0;
+ goto out;
-crtc_fail:
- drm_mode_config_cleanup(drm);
+out:
return ret;
}
-static void malidp_fini(struct drm_device *drm)
-{
- drm_mode_config_cleanup(drm);
-}
-
static int malidp_irq_init(struct platform_device *pdev)
{
int irq_de, irq_se, ret = 0;
@@ -879,7 +873,6 @@ static int malidp_bind(struct device *dev)
bind_fail:
of_node_put(malidp->crtc.port);
malidp->crtc.port = NULL;
- malidp_fini(drm);
query_hw_fail:
pm_runtime_put(dev);
if (pm_runtime_enabled(dev))
@@ -907,7 +900,6 @@ static void malidp_unbind(struct device *dev)
component_unbind_all(dev, drm);
of_node_put(malidp->crtc.port);
malidp->crtc.port = NULL;
- malidp_fini(drm);
pm_runtime_put(dev);
if (pm_runtime_enabled(dev))
pm_runtime_disable(dev);
--
2.37.3
[PATCH drm-misc-next v2 8/9] drm/arm/malidp: drv: protect device resources after removal
(Hardware) resources which are bound to the driver and device lifecycle
must not be accessed after the device and driver are unbound.
However, the DRM device isn't freed as long as the last user didn't
close it, hence userspace can still call into the driver.
Therefore protect the critical sections which are accessing those
resources with drm_dev_enter() and drm_dev_exit().
Signed-off-by: Danilo Krummrich
---
drivers/gpu/drm/arm/malidp_drv.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index aedd30f5f451..8bb8e8d14461 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -234,9 +234,12 @@ static void malidp_atomic_commit_tail(struct
drm_atomic_state *state)
struct malidp_drm *malidp = drm_to_malidp(drm);
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state;
- int i;
+ int i, idx;
bool fence_cookie = dma_fence_begin_signalling();
+ if (!drm_dev_enter(drm, &idx))
+ return;
+
pm_runtime_get_sync(drm->dev);
/*
@@ -267,6 +270,8 @@ static void malidp_atomic_commit_tail(struct
drm_atomic_state *state)
pm_runtime_put(drm->dev);
drm_atomic_helper_cleanup_planes(drm, state);
+
+ drm_dev_exit(idx);
}
static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
--
2.37.3
Re: [v5] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280
On Sat, 1 Oct 2022 at 17:25, Kalyan Thota wrote: > > > >-Original Message- > >From: Dmitry Baryshkov > >Sent: Friday, September 30, 2022 1:59 PM > >To: Doug Anderson ; Kalyan Thota (QUIC) > > > >Cc: y...@qualcomm.com; dri-devel ; > >linux-arm- > >msm ; freedreno > >; open list:OPEN FIRMWARE AND FLATTENED > >DEVICE TREE BINDINGS ; LKML >ker...@vger.kernel.org>; Rob Clark ; Stephen Boyd > >; Vinod Polimera (QUIC) > >; Abhinav Kumar (QUIC) > > > >Subject: Re: [v5] drm/msm/disp/dpu1: add support for dspp sub block flush in > >sc7280 > > > >WARNING: This email originated from outside of Qualcomm. Please be wary of > >any links or attachments, and do not enable macros. > > > >On 29 September 2022 19:13:20 GMT+03:00, Doug Anderson > > wrote: > >>Hi, > >> > >>On Wed, Sep 14, 2022 at 5:16 AM Kalyan Thota > >wrote: > >>> > >>> Flush mechanism for DSPP blocks has changed in sc7280 family, it > >>> allows individual sub blocks to be flushed in coordination with > >>> master flush control. > >>> > >>> Representation: master_flush && (PCC_flush | IGC_flush .. etc ) > >>> > >>> This change adds necessary support for the above design. > >>> > >>> Changes in v1: > >>> - Few nits (Doug, Dmitry) > >>> - Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry) > >>> > >>> Changes in v2: > >>> - Move the address offset to flush macro (Dmitry) > >>> - Seperate ops for the sub block flush (Dmitry) > >>> > >>> Changes in v3: > >>> - Reuse the DPU_DSPP_xx enum instead of a new one (Dmitry) > >>> > >>> Changes in v4: > >>> - Use shorter version for unsigned int (Stephen) > >>> > >>> Reviewed-by: Dmitry Baryshkov > >>> --- > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +- > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++- > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++ > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 > >-- > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++-- > >>> 5 files changed, 50 insertions(+), 6 deletions(-) > >> > >>Breadcrumbs: though this is tagged in the subject as v5 I think the > >>newest version is actually "resend v4" [1] which just fixes the > >>Signed-off-by. > > > >Not to mention that v5 misses the S-o-B tag. > > > >> > >>[1] > >>https://lore.kernel.org/r/1663825463-6715-1-git-send-email-quic_kalyant > >>@quicinc.com > > > Latest one is > https://lore.kernel.org/r/1663825463-6715-1-git-send-email-quic_kaly...@quicinc.com > that I last posted. > Don’t recollect on why tag was marked as v5. To avoid confusion, shall I > resend it again ? Currently I see v5 and after that comes a resend of v4. So, please send v6 with all the tags being present, no y@ in the msg-id/in-reply-to/etc. -- With best wishes Dmitry
Re: [PATCH v4 14/30] drm/client: Add some tests for drm_connector_pick_cmdline_mode()
On 9/29/22 13:31, Maxime Ripard wrote:
> drm_connector_pick_cmdline_mode() is in charge of finding a proper
> drm_display_mode from the definition we got in the video= command line
> argument.
>
> Let's add some unit tests to make sure we're not getting any regressions
> there.
>
> Signed-off-by: Maxime Ripard
>
> ---
> Changes in v4:
> - Removed MODULE macros
> ---
> drivers/gpu/drm/drm_client_modeset.c| 4 +
> drivers/gpu/drm/tests/drm_client_modeset_test.c | 105
>
> 2 files changed, 109 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c
> b/drivers/gpu/drm/drm_client_modeset.c
> index bbc535cc50dd..d553e793e673 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -1237,3 +1237,7 @@ int drm_client_modeset_dpms(struct drm_client_dev
> *client, int mode)
> return ret;
> }
> EXPORT_SYMBOL(drm_client_modeset_dpms);
> +
> +#ifdef CONFIG_DRM_KUNIT_TEST
> +#include "tests/drm_client_modeset_test.c"
> +#endif
> diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c
> b/drivers/gpu/drm/tests/drm_client_modeset_test.c
> new file mode 100644
> index ..09dc5acbbc42
> --- /dev/null
> +++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 Maxime Ripard
> + */
> +
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "drm_kunit_helpers.h"
> +
> +struct drm_client_modeset_test_priv {
> + struct drm_device *drm;
> + struct drm_connector connector;
> +};
> +
> +static int drm_client_modeset_connector_get_modes(struct drm_connector
> *connector)
> +{
> + struct drm_display_mode *mode;
Small nit: I guess this should be added on patch 21/30, as it is not
being currently used.
> + int count;
> +
> + count = drm_add_modes_noedid(connector, 1920, 1200);
> +
> + return count;
> +}
> +
> +static const struct drm_connector_helper_funcs
> drm_client_modeset_connector_helper_funcs = {
> + .get_modes = drm_client_modeset_connector_get_modes,
> +};
> +
> +static const struct drm_connector_funcs drm_client_modeset_connector_funcs =
> {
> +};
> +
> +static int drm_client_modeset_test_init(struct kunit *test)
> +{
> + struct drm_client_modeset_test_priv *priv;
> + int ret;
> +
> + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
I believe it would be nicer to use KUNIT_ASSERT_NOT_NULL here, instead
of returning a error.
> + test->priv = priv;
> +
> + priv->drm = drm_kunit_device_init(test, "drm-client-modeset-test");
> + if (IS_ERR(priv->drm))
> + return PTR_ERR(priv->drm);
Here you could use KUNIT_ASSERT_NOT_ERR_OR_NULL.
> +
> + ret = drmm_connector_init(priv->drm, &priv->connector,
> + &drm_client_modeset_connector_funcs,
> + DRM_MODE_CONNECTOR_Unknown,
> + NULL);
> + if (ret)
> + return ret;
Same idea here. This would make it more compliant to the KUnit API.
> + drm_connector_helper_add(&priv->connector,
> &drm_client_modeset_connector_helper_funcs);
> +
> + return 0;
> +
> +}
> +
> +static void drm_pick_cmdline_res_1920_1080_60(struct kunit *test)
> +{
> + struct drm_client_modeset_test_priv *priv = test->priv;
> + struct drm_device *drm = priv->drm;
> + struct drm_connector *connector = &priv->connector;
> + struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode;
> + struct drm_display_mode *expected_mode, *mode;
> + const char *cmdline = "1920x1080@60";
> + int ret;
> +
> + expected_mode = drm_mode_find_dmt(priv->drm, 1920, 1080, 60, false);
> + KUNIT_ASSERT_PTR_NE(test, expected_mode, NULL);
You could use KUNIT_ASSERT_NOT_NULL here.
> +
> + KUNIT_ASSERT_TRUE(test,
> + drm_mode_parse_command_line_for_connector(cmdline,
> + connector,
> +
> cmdline_mode));
> +
> + mutex_lock(&drm->mode_config.mutex);
> + ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080);
> + mutex_unlock(&drm->mode_config.mutex);
> + KUNIT_ASSERT_GT(test, ret, 0);
> +
> + mode = drm_connector_pick_cmdline_mode(connector);
> + KUNIT_ASSERT_PTR_NE(test, mode, NULL);
You could also use KUNIT_ASSERT_NOT_NULL here.
This idea could also apply to the patches 21/30 and 22/30, which have a
similar structure and are also using KUNIT_ASSERT_PTR_NE.
Best Regards,
- Maíra Canal
> +
> + KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode));
> +}
> +
> +static struct kunit_case drm_pick_cmdline_tests[] = {
> + KUNIT_CASE(drm_pick_cmdline_res_1920_1080_60),
> + {}
> +};
> +
> +static stru
[PATCH 0/5] drm: Fix math issues in MSM DSC implementation
Various removals of complex yet unnecessary math, fixing all uses of drm_dsc_config::bits_per_pixel to deal with the fact that this field includes four fractional bits, and finally an approach for dealing with dsi_host setting negative values in range_bpg_offset, resulting in overflow inside drm_dsc_pps_payload_pack(). Note that updating the static bpg_offset array to limit the size of these negative values to 6 bits changes what would be written to the DPU hardware at register(s) DSC_RANGE_BPG_OFFSET, hence the choice has been made to cover up for this while packing the value into a smaller field instead. Altogether this series is responsible for solving _all_ Display Stream Compression issues and artifacts on the Sony Tama (sdm845) Akatsuki smartphone (2880x1440p). Marijn Suijten (5): drm/msm/dsi: Remove useless math in DSC calculation drm/msm/dsi: Remove repeated calculation of slice_per_intf drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional bits drm/dsc: Prevent negative BPG offsets from shadowing adjacent bitfields drivers/gpu/drm/display/drm_dsc_helper.c | 6 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 11 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 45 ++ 3 files changed, 33 insertions(+), 29 deletions(-) -- 2.37.3
[PATCH 1/5] drm/msm/dsi: Remove useless math in DSC calculation
Multiplying a value by 2 and adding 1 to it always results in a value
that is uneven, and that 1 gets truncated immediately when performing
integer division by 2 again. There is no "rounding" possible here.
Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 8e4bc586c262..e05bae647431 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1864,12 +1864,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config
*dsc)
data = 2048 * (dsc->rc_model_size - dsc->initial_offset +
num_extra_mux_bits);
dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
- /* bpp * 16 + 0.5 */
- data = dsc->bits_per_pixel * 16;
- data *= 2;
- data++;
- data /= 2;
- target_bpp_x16 = data;
+ target_bpp_x16 = dsc->bits_per_pixel * 16;
data = (dsc->initial_xmit_delay * target_bpp_x16) / 16;
final_value = dsc->rc_model_size - data + num_extra_mux_bits;
--
2.37.3
[PATCH 2/5] drm/msm/dsi: Remove repeated calculation of slice_per_intf
slice_per_intf is already computed for intf_width, which holds the same
value as hdisplay.
Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Signed-off-by: Marijn Suijten
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index e05bae647431..cb6f2fa11f58 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -842,7 +842,7 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host,
bool enable,
static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool
is_cmd_mode, u32 hdisplay)
{
struct drm_dsc_config *dsc = msm_host->dsc;
- u32 reg, intf_width, reg_ctrl, reg_ctrl2;
+ u32 reg, reg_ctrl, reg_ctrl2;
u32 slice_per_intf, total_bytes_per_intf;
u32 pkt_per_line;
u32 bytes_in_slice;
@@ -851,8 +851,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
/* first calculate dsc parameters and then program
* compress mode registers
*/
- intf_width = hdisplay;
- slice_per_intf = DIV_ROUND_UP(intf_width, dsc->slice_width);
+ slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width);
/* If slice_per_pkt is greater than slice_per_intf
* then default to 1. This can happen during partial
@@ -861,7 +860,6 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
if (slice_per_intf > dsc->slice_count)
dsc->slice_count = 1;
- slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width);
bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel,
8);
dsc->slice_chunk_size = bytes_in_slice;
--
2.37.3
[PATCH 4/5] drm/msm/dpu1: Account for DSC's bits_per_pixel having 4 fractional bits
According to the comment this DPU register contains the bits per pixel
as a 6.4 fractional value, conveniently matching the contents of
bits_per_pixel in struct drm_dsc_config which also uses 4 fractional
bits. However, the downstream source this implementation was
copy-pasted from has its bpp field stored _without_ fractional part.
This makes the entire convoluted math obsolete as it is impossible to
pull those 4 fractional bits out of thin air, by somehow trying to reuse
the lowest 2 bits of a non-fractional bpp (lsb = bpp % 4??).
The rest of the code merely attempts to keep the integer part a multiple
of 4, which is rendered useless thanks to data |= dsc->bits_per_pixel <<
12; already filling up those bits anyway (but not on downstream).
Fixes: c110cfd1753e ("drm/msm/disp/dpu1: Add support for DSC")
Signed-off-by: Marijn Suijten
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
index f2ddcfb6f7ee..3662df698dae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -42,7 +42,7 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
u32 initial_lines)
{
struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
- u32 data, lsb, bpp;
+ u32 data;
u32 slice_last_group_size;
u32 det_thresh_flatness;
bool is_cmd_mode = !(mode & DSC_MODE_VIDEO);
@@ -56,14 +56,7 @@ static void dpu_hw_dsc_config(struct dpu_hw_dsc *hw_dsc,
data = (initial_lines << 20);
data |= ((slice_last_group_size - 1) << 18);
/* bpp is 6.4 format, 4 LSBs bits are for fractional part */
- data |= dsc->bits_per_pixel << 12;
- lsb = dsc->bits_per_pixel % 4;
- bpp = dsc->bits_per_pixel / 4;
- bpp *= 4;
- bpp <<= 4;
- bpp |= lsb;
-
- data |= bpp << 8;
+ data |= (dsc->bits_per_pixel << 8);
data |= (dsc->block_pred_enable << 7);
data |= (dsc->line_buf_depth << 3);
data |= (dsc->simple_422 << 2);
--
2.37.3
[PATCH 3/5] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits
drm_dsc_config's bits_per_pixel field holds a fractional value with 4
bits, which all panel drivers should adhere to for
drm_dsc_pps_payload_pack() to generate a valid payload. All code in the
DSI driver here seems to assume that this field doesn't contain any
fractional bits, hence resulting in the wrong values being computed.
Since none of the calculations leave any room for fractional bits or
seem to indicate any possible area of support, disallow such values
altogether.
Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 34 +++---
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index cb6f2fa11f58..42a5c9776f52 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -847,6 +847,11 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
u32 pkt_per_line;
u32 bytes_in_slice;
u32 eol_byte_num;
+ int bpp = dsc->bits_per_pixel >> 4;
+
+ if (dsc->bits_per_pixel & 0xf)
+ /* dsi_populate_dsc_params() already caught this case */
+ pr_err("DSI does not support fractional bits_per_pixel\n");
/* first calculate dsc parameters and then program
* compress mode registers
@@ -860,7 +865,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
if (slice_per_intf > dsc->slice_count)
dsc->slice_count = 1;
- bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel,
8);
+ bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
dsc->slice_chunk_size = bytes_in_slice;
@@ -913,6 +918,7 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host,
bool is_bonded_dsi)
u32 va_end = va_start + mode->vdisplay;
u32 hdisplay = mode->hdisplay;
u32 wc;
+ int ret;
DBG("");
@@ -948,7 +954,9 @@ static void dsi_timing_setup(struct msm_dsi_host *msm_host,
bool is_bonded_dsi)
/* we do the calculations for dsc parameters here so that
* panel can use these parameters
*/
- dsi_populate_dsc_params(dsc);
+ ret = dsi_populate_dsc_params(dsc);
+ if (ret)
+ return;
/* Divide the display by 3 but keep back/font porch and
* pulse width same
@@ -1229,6 +1237,10 @@ static int dsi_cmd_dma_add(struct msm_dsi_host *msm_host,
if (packet.size < len)
memset(data + packet.size, 0xff, len - packet.size);
+ if (msg->type == MIPI_DSI_PICTURE_PARAMETER_SET)
+ print_hex_dump(KERN_DEBUG, "ALL:", DUMP_PREFIX_NONE,
+ 16, 1, data, len, false);
+
if (cfg_hnd->ops->tx_buf_put)
cfg_hnd->ops->tx_buf_put(msm_host);
@@ -1786,6 +1798,12 @@ static int dsi_populate_dsc_params(struct drm_dsc_config
*dsc)
int data;
int final_value, final_scale;
int i;
+ int bpp = dsc->bits_per_pixel >> 4;
+
+ if (dsc->bits_per_pixel & 0xf) {
+ pr_err("DSI does not support fractional bits_per_pixel\n");
+ return -EINVAL;
+ }
dsc->rc_model_size = 8192;
dsc->first_line_bpg_offset = 12;
@@ -1807,7 +1825,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config
*dsc)
}
dsc->initial_offset = 6144; /* Not bpp 12 */
- if (dsc->bits_per_pixel != 8)
+ if (bpp != 8)
dsc->initial_offset = 2048; /* bpp = 12 */
mux_words_size = 48;/* bpc == 8/10 */
@@ -1830,16 +1848,16 @@ static int dsi_populate_dsc_params(struct
drm_dsc_config *dsc)
* params are calculated
*/
groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
- dsc->slice_chunk_size = dsc->slice_width * dsc->bits_per_pixel / 8;
- if ((dsc->slice_width * dsc->bits_per_pixel) % 8)
+ dsc->slice_chunk_size = dsc->slice_width * bpp / 8;
+ if ((dsc->slice_width * bpp) % 8)
dsc->slice_chunk_size++;
/* rbs-min */
min_rate_buffer_size = dsc->rc_model_size - dsc->initial_offset +
- dsc->initial_xmit_delay * dsc->bits_per_pixel +
+ dsc->initial_xmit_delay * bpp +
groups_per_line * dsc->first_line_bpg_offset;
- hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->bits_per_pixel);
+ hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, bpp);
dsc->initial_dec_delay = hrd_delay - dsc->initial_xmit_delay;
@@ -1862,7 +1880,7 @@ static int dsi_populate_dsc_params(struct drm_dsc_config
*dsc)
data = 2048 * (dsc->rc_model_size - dsc->initial_offset +
num_extra_mux_bits);
dsc->slice_
[PATCH 5/5] drm/dsc: Prevent negative BPG offsets from shadowing adjacent bitfields
msm's dsi_host specifies negative BPG offsets which fill the full 8 bits
of a char thanks to two's complement: this however results in those bits
bleeding into the next parameter when the field is only expected to
contain 6-bit wide values.
As a consequence random slices appear corrupted on-screen (tested on a
Sony Tama Akatsuki device with sdm845).
Use AND operators to limit all values that constitute the RC Range
parameter fields to their expected size.
Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
Signed-off-by: Marijn Suijten
---
drivers/gpu/drm/display/drm_dsc_helper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
b/drivers/gpu/drm/display/drm_dsc_helper.c
index c869c6e51e2b..2e7ef242685d 100644
--- a/drivers/gpu/drm/display/drm_dsc_helper.c
+++ b/drivers/gpu/drm/display/drm_dsc_helper.c
@@ -243,11 +243,11 @@ void drm_dsc_pps_payload_pack(struct
drm_dsc_picture_parameter_set *pps_payload,
*/
for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
pps_payload->rc_range_parameters[i] =
- cpu_to_be16((dsc_cfg->rc_range_params[i].range_min_qp <<
+ cpu_to_be16(((dsc_cfg->rc_range_params[i].range_min_qp
& 0x1f) <<
DSC_PPS_RC_RANGE_MINQP_SHIFT) |
- (dsc_cfg->rc_range_params[i].range_max_qp <<
+ ((dsc_cfg->rc_range_params[i].range_max_qp
& 0x1f) <<
DSC_PPS_RC_RANGE_MAXQP_SHIFT) |
-
(dsc_cfg->rc_range_params[i].range_bpg_offset));
+
(dsc_cfg->rc_range_params[i].range_bpg_offset & 0x3f));
}
/* PPS 88 */
--
2.37.3
Re: [PATCH 1/5] drm/msm/dsi: Remove useless math in DSC calculation
On 1.10.2022 21:08, Marijn Suijten wrote:
> Multiplying a value by 2 and adding 1 to it always results in a value
> that is uneven, and that 1 gets truncated immediately when performing
> integer division by 2 again. There is no "rounding" possible here.
>
> Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
> Signed-off-by: Marijn Suijten
> ---
Reviewed-by: Konrad Dybcio
Konrad
> drivers/gpu/drm/msm/dsi/dsi_host.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 8e4bc586c262..e05bae647431 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1864,12 +1864,7 @@ static int dsi_populate_dsc_params(struct
> drm_dsc_config *dsc)
> data = 2048 * (dsc->rc_model_size - dsc->initial_offset +
> num_extra_mux_bits);
> dsc->slice_bpg_offset = DIV_ROUND_UP(data, groups_total);
>
> - /* bpp * 16 + 0.5 */
> - data = dsc->bits_per_pixel * 16;
> - data *= 2;
> - data++;
> - data /= 2;
> - target_bpp_x16 = data;
> + target_bpp_x16 = dsc->bits_per_pixel * 16;
>
> data = (dsc->initial_xmit_delay * target_bpp_x16) / 16;
> final_value = dsc->rc_model_size - data + num_extra_mux_bits;
Re: [PATCH 2/5] drm/msm/dsi: Remove repeated calculation of slice_per_intf
On 1.10.2022 21:08, Marijn Suijten wrote:
> slice_per_intf is already computed for intf_width, which holds the same
> value as hdisplay.
>
> Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
> Signed-off-by: Marijn Suijten
> ---
Reviewed-by: Konrad Dybcio
Konrad
> drivers/gpu/drm/msm/dsi/dsi_host.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index e05bae647431..cb6f2fa11f58 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -842,7 +842,7 @@ static void dsi_ctrl_config(struct msm_dsi_host
> *msm_host, bool enable,
> static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool
> is_cmd_mode, u32 hdisplay)
> {
> struct drm_dsc_config *dsc = msm_host->dsc;
> - u32 reg, intf_width, reg_ctrl, reg_ctrl2;
> + u32 reg, reg_ctrl, reg_ctrl2;
> u32 slice_per_intf, total_bytes_per_intf;
> u32 pkt_per_line;
> u32 bytes_in_slice;
> @@ -851,8 +851,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
> *msm_host, bool is_cmd_mod
> /* first calculate dsc parameters and then program
>* compress mode registers
>*/
> - intf_width = hdisplay;
> - slice_per_intf = DIV_ROUND_UP(intf_width, dsc->slice_width);
> + slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width);
>
> /* If slice_per_pkt is greater than slice_per_intf
>* then default to 1. This can happen during partial
> @@ -861,7 +860,6 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
> *msm_host, bool is_cmd_mod
> if (slice_per_intf > dsc->slice_count)
> dsc->slice_count = 1;
>
> - slice_per_intf = DIV_ROUND_UP(hdisplay, dsc->slice_width);
> bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel,
> 8);
>
> dsc->slice_chunk_size = bytes_in_slice;
Re: [PATCH 5/5] drm/dsc: Prevent negative BPG offsets from shadowing adjacent bitfields
On 2022-10-01 21:08:07, Marijn Suijten wrote:
> msm's dsi_host specifies negative BPG offsets which fill the full 8 bits
> of a char thanks to two's complement: this however results in those bits
> bleeding into the next parameter when the field is only expected to
> contain 6-bit wide values.
> As a consequence random slices appear corrupted on-screen (tested on a
> Sony Tama Akatsuki device with sdm845).
>
> Use AND operators to limit all values that constitute the RC Range
> parameter fields to their expected size.
>
> Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
> Signed-off-by: Marijn Suijten
> ---
> drivers/gpu/drm/display/drm_dsc_helper.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dsc_helper.c
> b/drivers/gpu/drm/display/drm_dsc_helper.c
> index c869c6e51e2b..2e7ef242685d 100644
> --- a/drivers/gpu/drm/display/drm_dsc_helper.c
> +++ b/drivers/gpu/drm/display/drm_dsc_helper.c
> @@ -243,11 +243,11 @@ void drm_dsc_pps_payload_pack(struct
> drm_dsc_picture_parameter_set *pps_payload,
>*/
> for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> pps_payload->rc_range_parameters[i] =
> - cpu_to_be16((dsc_cfg->rc_range_params[i].range_min_qp <<
> + cpu_to_be16(((dsc_cfg->rc_range_params[i].range_min_qp
> & 0x1f) <<
>DSC_PPS_RC_RANGE_MINQP_SHIFT) |
> - (dsc_cfg->rc_range_params[i].range_max_qp <<
> + ((dsc_cfg->rc_range_params[i].range_max_qp
> & 0x1f) <<
>DSC_PPS_RC_RANGE_MAXQP_SHIFT) |
> -
> (dsc_cfg->rc_range_params[i].range_bpg_offset));
> +
> (dsc_cfg->rc_range_params[i].range_bpg_offset & 0x3f));
Pre-empting the reviews: I was contemplating whether to use FIELD_PREP
here, given that it's not yet used anywhere else in this file. For that
I'd remove the existing _SHIFT definitions and replace them with:
#define DSC_PPS_RC_RANGE_MINQP_MASK GENMASK(15, 11)
#define DSC_PPS_RC_RANGE_MAXQP_MASK GENMASK(10, 6)
#define DSC_PPS_RC_RANGE_BPG_OFFSET_MASKGENMASK(5, 0)
And turn this section of code into:
cpu_to_be16(FIELD_PREP(DSC_PPS_RC_RANGE_MINQP_MASK,
dsc_cfg->rc_range_params[i].range_min_qp) |
FIELD_PREP(DSC_PPS_RC_RANGE_MAXQP_MASK,
dsc_cfg->rc_range_params[i].range_max_qp) |
FIELD_PREP(DSC_PPS_RC_RANGE_BPG_OFFSET_MASK,
dsc_cfg->rc_range_params[i].range_bpg_offset));
Is that okay/recommended?
- Marijn
> }
>
> /* PPS 88 */
> --
> 2.37.3
>
Re: [PATCH 3/5] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits
On 1.10.2022 21:08, Marijn Suijten wrote:
> drm_dsc_config's bits_per_pixel field holds a fractional value with 4
> bits, which all panel drivers should adhere to for
> drm_dsc_pps_payload_pack() to generate a valid payload. All code in the
> DSI driver here seems to assume that this field doesn't contain any
> fractional bits, hence resulting in the wrong values being computed.
> Since none of the calculations leave any room for fractional bits or
> seem to indicate any possible area of support, disallow such values
> altogether.
>
> Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
> Signed-off-by: Marijn Suijten
> ---
Reviewed-by: Konrad Dybcio
Konrad
> drivers/gpu/drm/msm/dsi/dsi_host.c | 34 +++---
> 1 file changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index cb6f2fa11f58..42a5c9776f52 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -847,6 +847,11 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
> *msm_host, bool is_cmd_mod
> u32 pkt_per_line;
> u32 bytes_in_slice;
> u32 eol_byte_num;
> + int bpp = dsc->bits_per_pixel >> 4;
> +
> + if (dsc->bits_per_pixel & 0xf)
> + /* dsi_populate_dsc_params() already caught this case */
> + pr_err("DSI does not support fractional bits_per_pixel\n");
>
> /* first calculate dsc parameters and then program
>* compress mode registers
> @@ -860,7 +865,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
> *msm_host, bool is_cmd_mod
> if (slice_per_intf > dsc->slice_count)
> dsc->slice_count = 1;
>
> - bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel,
> 8);
> + bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
>
> dsc->slice_chunk_size = bytes_in_slice;
>
> @@ -913,6 +918,7 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
> u32 va_end = va_start + mode->vdisplay;
> u32 hdisplay = mode->hdisplay;
> u32 wc;
> + int ret;
>
> DBG("");
>
> @@ -948,7 +954,9 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
> /* we do the calculations for dsc parameters here so that
>* panel can use these parameters
>*/
> - dsi_populate_dsc_params(dsc);
> + ret = dsi_populate_dsc_params(dsc);
> + if (ret)
> + return;
>
> /* Divide the display by 3 but keep back/font porch and
>* pulse width same
> @@ -1229,6 +1237,10 @@ static int dsi_cmd_dma_add(struct msm_dsi_host
> *msm_host,
> if (packet.size < len)
> memset(data + packet.size, 0xff, len - packet.size);
>
> + if (msg->type == MIPI_DSI_PICTURE_PARAMETER_SET)
> + print_hex_dump(KERN_DEBUG, "ALL:", DUMP_PREFIX_NONE,
> + 16, 1, data, len, false);
> +
> if (cfg_hnd->ops->tx_buf_put)
> cfg_hnd->ops->tx_buf_put(msm_host);
>
> @@ -1786,6 +1798,12 @@ static int dsi_populate_dsc_params(struct
> drm_dsc_config *dsc)
> int data;
> int final_value, final_scale;
> int i;
> + int bpp = dsc->bits_per_pixel >> 4;
> +
> + if (dsc->bits_per_pixel & 0xf) {
> + pr_err("DSI does not support fractional bits_per_pixel\n");
> + return -EINVAL;
> + }
>
> dsc->rc_model_size = 8192;
> dsc->first_line_bpg_offset = 12;
> @@ -1807,7 +1825,7 @@ static int dsi_populate_dsc_params(struct
> drm_dsc_config *dsc)
> }
>
> dsc->initial_offset = 6144; /* Not bpp 12 */
> - if (dsc->bits_per_pixel != 8)
> + if (bpp != 8)
> dsc->initial_offset = 2048; /* bpp = 12 */
>
> mux_words_size = 48;/* bpc == 8/10 */
> @@ -1830,16 +1848,16 @@ static int dsi_populate_dsc_params(struct
> drm_dsc_config *dsc)
>* params are calculated
>*/
> groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
> - dsc->slice_chunk_size = dsc->slice_width * dsc->bits_per_pixel / 8;
> - if ((dsc->slice_width * dsc->bits_per_pixel) % 8)
> + dsc->slice_chunk_size = dsc->slice_width * bpp / 8;
> + if ((dsc->slice_width * bpp) % 8)
> dsc->slice_chunk_size++;
>
> /* rbs-min */
> min_rate_buffer_size = dsc->rc_model_size - dsc->initial_offset +
> - dsc->initial_xmit_delay * dsc->bits_per_pixel +
> + dsc->initial_xmit_delay * bpp +
> groups_per_line * dsc->first_line_bpg_offset;
>
> - hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, dsc->bits_per_pixel);
> + hrd_delay = DIV_ROUND_UP(min_rate_buffer_size, bpp);
>
> dsc->initial_dec_delay = hrd_delay - dsc->initial_xmit_delay;
>
Re: [PATCH 3/5] drm/msm/dsi: Account for DSC's bits_per_pixel having 4 fractional bits
Doing some self-review as these patches accrued some bit-rot while
waiting to be sent.
On 2022-10-01 21:08:05, Marijn Suijten wrote:
> drm_dsc_config's bits_per_pixel field holds a fractional value with 4
> bits, which all panel drivers should adhere to for
> drm_dsc_pps_payload_pack() to generate a valid payload. All code in the
> DSI driver here seems to assume that this field doesn't contain any
> fractional bits, hence resulting in the wrong values being computed.
> Since none of the calculations leave any room for fractional bits or
> seem to indicate any possible area of support, disallow such values
> altogether.
>
> Fixes: b9080324d6ca ("drm/msm/dsi: add support for dsc data")
> Signed-off-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 34 +++---
> 1 file changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index cb6f2fa11f58..42a5c9776f52 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -847,6 +847,11 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
> *msm_host, bool is_cmd_mod
> u32 pkt_per_line;
> u32 bytes_in_slice;
> u32 eol_byte_num;
> + int bpp = dsc->bits_per_pixel >> 4;
This should have been u16 instead of int.
> +
> + if (dsc->bits_per_pixel & 0xf)
Should there be a define for this mask?
> + /* dsi_populate_dsc_params() already caught this case */
> + pr_err("DSI does not support fractional bits_per_pixel\n");
This file mostly uses pr_err(), but it's probably cleaner to use
DRM_DEV_ERROR(&msm_host->pdev->dev, ...) even though it's not prevalent
yet?
>
> /* first calculate dsc parameters and then program
>* compress mode registers
> @@ -860,7 +865,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
> *msm_host, bool is_cmd_mod
> if (slice_per_intf > dsc->slice_count)
> dsc->slice_count = 1;
>
> - bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * dsc->bits_per_pixel,
> 8);
> + bytes_in_slice = DIV_ROUND_UP(dsc->slice_width * bpp, 8);
>
> dsc->slice_chunk_size = bytes_in_slice;
>
> @@ -913,6 +918,7 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
> u32 va_end = va_start + mode->vdisplay;
> u32 hdisplay = mode->hdisplay;
> u32 wc;
> + int ret;
>
> DBG("");
>
> @@ -948,7 +954,9 @@ static void dsi_timing_setup(struct msm_dsi_host
> *msm_host, bool is_bonded_dsi)
> /* we do the calculations for dsc parameters here so that
>* panel can use these parameters
>*/
> - dsi_populate_dsc_params(dsc);
> + ret = dsi_populate_dsc_params(dsc);
> + if (ret)
> + return;
>
> /* Divide the display by 3 but keep back/font porch and
>* pulse width same
> @@ -1229,6 +1237,10 @@ static int dsi_cmd_dma_add(struct msm_dsi_host
> *msm_host,
> if (packet.size < len)
> memset(data + packet.size, 0xff, len - packet.size);
>
> + if (msg->type == MIPI_DSI_PICTURE_PARAMETER_SET)
> + print_hex_dump(KERN_DEBUG, "ALL:", DUMP_PREFIX_NONE,
> + 16, 1, data, len, false);
> +
> if (cfg_hnd->ops->tx_buf_put)
> cfg_hnd->ops->tx_buf_put(msm_host);
>
> @@ -1786,6 +1798,12 @@ static int dsi_populate_dsc_params(struct
> drm_dsc_config *dsc)
> int data;
> int final_value, final_scale;
> int i;
> + int bpp = dsc->bits_per_pixel >> 4;
Same u16 here.
- Marijn
> +
> + if (dsc->bits_per_pixel & 0xf) {
> + pr_err("DSI does not support fractional bits_per_pixel\n");
> + return -EINVAL;
> + }
>
> dsc->rc_model_size = 8192;
> dsc->first_line_bpg_offset = 12;
> @@ -1807,7 +1825,7 @@ static int dsi_populate_dsc_params(struct
> drm_dsc_config *dsc)
> }
>
> dsc->initial_offset = 6144; /* Not bpp 12 */
> - if (dsc->bits_per_pixel != 8)
> + if (bpp != 8)
> dsc->initial_offset = 2048; /* bpp = 12 */
>
> mux_words_size = 48;/* bpc == 8/10 */
> @@ -1830,16 +1848,16 @@ static int dsi_populate_dsc_params(struct
> drm_dsc_config *dsc)
>* params are calculated
>*/
> groups_per_line = DIV_ROUND_UP(dsc->slice_width, 3);
> - dsc->slice_chunk_size = dsc->slice_width * dsc->bits_per_pixel / 8;
> - if ((dsc->slice_width * dsc->bits_per_pixel) % 8)
> + dsc->slice_chunk_size = dsc->slice_width * bpp / 8;
> + if ((dsc->slice_width * bpp) % 8)
> dsc->slice_chunk_size++;
>
> /* rbs-min */
> min_rate_buffer_size = dsc->rc_model_size - dsc->initial_offset +
> - dsc->initial_xmit_delay * dsc->bits_per_pixel +
> + dsc->initial_xmit_d
Re: [PATCH 4/4] drm: panel: Add lg sw43408 panel driver
On 2022-07-18 22:30:51, Caleb Connolly wrote:
> From: Sumit Semwal
>
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel, used in some Pixel3
> phones.
>
> Whatever init sequence we have for this panel isn't capable of
> initialising it completely, toggling the reset gpio ever causes the
> panel to die. Until this is resolved we avoid resetting the panel. The
> disable/unprepare functions only put the panel to sleep mode and
> disable the backlight.
>
> Signed-off-by: Sumit Semwal
> [vinod: Add DSC support]
> Signed-off-by: Vinod Koul
> [caleb: cleanup and support turning off the panel]
> Signed-off-by: Caleb Connolly
> ---
> MAINTAINERS | 8 +
> drivers/gpu/drm/panel/Kconfig| 11 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-lg-sw43408.c | 586 +++
> 4 files changed, 606 insertions(+)
> create mode 100644 drivers/gpu/drm/panel/panel-lg-sw43408.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f679152bdbad..8a2b954ad140 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6376,6 +6376,14 @@ S: Orphan / Obsolete
> F: drivers/gpu/drm/i810/
> F: include/uapi/drm/i810_drm.h
>
> +DRM DRIVER FOR LG SW43408 PANELS
> +M: Sumit Semwal
> +M: Caleb Connolly
> +S: Maintained
> +T: git git://anongit.freedesktop.org/drm/drm-misc
> +F: Documentation/devicetree/bindings/display/panel/lg,sw43408-panel.txt
> +F: drivers/gpu/drm/panel/panel-lg-sw43408.c
> +
> DRM DRIVER FOR LVDS PANELS
> M: Laurent Pinchart
> L: dri-devel@lists.freedesktop.org
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 38799effd00a..706b112794b9 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -256,6 +256,17 @@ config DRM_PANEL_LEADTEK_LTK500HD1829
> 24 bit RGB per pixel. It provides a MIPI DSI interface to
> the host and has a built-in LED backlight.
>
> +config DRM_PANEL_LG_SW43408
> + tristate "LG SW43408 panel"
> + depends on OF
> + depends on DRM_MIPI_DSI
> + depends on BACKLIGHT_CLASS_DEVICE
> + help
> + Say Y here if you want to enable support for LG sw43408 panel.
> + The panel has a 1080x2160 resolution and uses
> + 24 bit RGB per pixel. It provides a MIPI DSI interface to
> + the host and has a built-in LED backlight.
> +
> config DRM_PANEL_SAMSUNG_LD9040
> tristate "Samsung LD9040 RGB/SPI panel"
> depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 42a7ab54234b..ba26a69b74e7 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) +=
> panel-leadtek-ltk050h3146w.o
> obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
> obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
> obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> +obj-$(CONFIG_DRM_PANEL_LG_SW43408) += panel-lg-sw43408.o
> obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
> obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
> obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
> diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c
> b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> new file mode 100644
> index ..c7b8ec7b970d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Linaro Ltd
> + * Author: Sumit Semwal
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +struct panel_cmd {
> + size_t len;
> + const char *data;
> +};
> +
> +#define _INIT_CMD(...) \
> + {\
> + .len = sizeof((char[]){ __VA_ARGS__ }), .data = (char[]) \
> + {\
> + __VA_ARGS__ \
> + }\
> + }
> +
> +static const char *const regulator_names[] = {
> + "vddi",
> + "vpnl",
> +};
> +
> +static const unsigned long regulator_enable_loads[] = {
> + 62000,
> + 857000,
> +};
> +
> +static const unsigned long regulator_disable_loads[] = {
> + 80,
> + 0,
> +};
> +
> +struct sw43408_panel {
> + struct drm_panel base;
> + struct mipi_dsi_device *link;
> +
> + const struct drm_display_mode *mode;
> + struct backlight_device *backlight;
> +
> + struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
> +
> + struct gpio_
[PATCH] drm: rcar-du: Fix Kconfig dependency between RCAR_DU and RCAR_MIPI_DSI
When the R-Car MIPI DSI driver was added, it was a standalone encoder
driver without any dependency to or from the R-Car DU driver. Commit
957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence") then
added a direct call from the DU driver to the MIPI DSI driver, without
updating Kconfig to take the new dependency into account. Fix it the
same way that the LVDS encoder is handled.
Fixes: 957fe62d7d15 ("drm: rcar-du: Fix DSI enable & disable sequence")
Reported-by: kernel test robot
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/rcar-du/Kconfig | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
index c959e8c6be7d..fd2c2eaee26b 100644
--- a/drivers/gpu/drm/rcar-du/Kconfig
+++ b/drivers/gpu/drm/rcar-du/Kconfig
@@ -44,12 +44,17 @@ config DRM_RCAR_LVDS
select OF_FLATTREE
select OF_OVERLAY
+config DRM_RCAR_USE_MIPI_DSI
+ bool "R-Car DU MIPI DSI Encoder Support"
+ depends on DRM_BRIDGE && OF
+ default DRM_RCAR_DU
+ help
+ Enable support for the R-Car Display Unit embedded MIPI DSI encoders.
+
config DRM_RCAR_MIPI_DSI
- tristate "R-Car DU MIPI DSI Encoder Support"
- depends on DRM && DRM_BRIDGE && OF
+ def_tristate DRM_RCAR_DU
+ depends on DRM_RCAR_USE_MIPI_DSI
select DRM_MIPI_DSI
- help
- Enable support for the R-Car Display Unit embedded MIPI DSI encoders.
config DRM_RCAR_VSP
bool "R-Car DU VSP Compositor Support" if ARM
base-commit: 7860d720a84c74b2761c6b7995392a798ab0a3cb
--
Regards,
Laurent Pinchart
[PATCH v3 1/2] drm/tests: Split drm_test_dp_mst_calc_pbn_mode into parameterized tests
The drm_test_dp_mst_calc_pbn_mode is based on a loop that executes tests
for a couple of test cases. This could be better represented by
parameterized tests, provided by KUnit.
So, convert the drm_test_dp_mst_calc_pbn_mode into parameterized tests.
Signed-off-by: Maíra Canal
Reviewed-by: Michał Winiarski
---
v1 -> v2:
https://lore.kernel.org/dri-devel/20220925222719.345424-1-mca...@igalia.com/T/#m056610a23a63109484afeafefb5846178c4d36b2
- Add Michał's Reviewed-by tag.
v2 -> v3:
https://lore.kernel.org/dri-devel/20220927221206.55930-1-mca...@igalia.com/T/#m2dc961da2d4921566cd0f9a8ed9d2d33a1cf4416
- No changes.
---
.../gpu/drm/tests/drm_dp_mst_helper_test.c| 77 +--
1 file changed, 53 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
index 65c9d225b558..12f41881db6b 100644
--- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
@@ -16,33 +16,62 @@
#include "../display/drm_dp_mst_topology_internal.h"
+struct drm_dp_mst_calc_pbn_mode_test {
+ const int clock;
+ const int bpp;
+ const bool dsc;
+ const int expected;
+};
+
+static const struct drm_dp_mst_calc_pbn_mode_test
drm_dp_mst_calc_pbn_mode_cases[] = {
+ {
+ .clock = 154000,
+ .bpp = 30,
+ .dsc = false,
+ .expected = 689
+ },
+ {
+ .clock = 234000,
+ .bpp = 30,
+ .dsc = false,
+ .expected = 1047
+ },
+ {
+ .clock = 297000,
+ .bpp = 24,
+ .dsc = false,
+ .expected = 1063
+ },
+ {
+ .clock = 332880,
+ .bpp = 24,
+ .dsc = true,
+ .expected = 50
+ },
+ {
+ .clock = 324540,
+ .bpp = 24,
+ .dsc = true,
+ .expected = 49
+ },
+};
+
static void drm_test_dp_mst_calc_pbn_mode(struct kunit *test)
{
- int pbn, i;
- const struct {
- int rate;
- int bpp;
- int expected;
- bool dsc;
- } test_params[] = {
- { 154000, 30, 689, false },
- { 234000, 30, 1047, false },
- { 297000, 24, 1063, false },
- { 332880, 24, 50, true },
- { 324540, 24, 49, true },
- };
-
- for (i = 0; i < ARRAY_SIZE(test_params); i++) {
- pbn = drm_dp_calc_pbn_mode(test_params[i].rate,
- test_params[i].bpp,
- test_params[i].dsc);
- KUNIT_EXPECT_EQ_MSG(test, pbn, test_params[i].expected,
- "Expected PBN %d for clock %d bpp %d, got
%d\n",
-test_params[i].expected, test_params[i].rate,
-test_params[i].bpp, pbn);
- }
+ const struct drm_dp_mst_calc_pbn_mode_test *params = test->param_value;
+
+ KUNIT_EXPECT_EQ(test, drm_dp_calc_pbn_mode(params->clock, params->bpp,
params->dsc),
+ params->expected);
}
+static void dp_mst_calc_pbn_mode_desc(const struct
drm_dp_mst_calc_pbn_mode_test *t, char *desc)
+{
+ sprintf(desc, "Clock %d BPP %d DSC %s", t->clock, t->bpp, t->dsc ?
"enabled" : "disabled");
+}
+
+KUNIT_ARRAY_PARAM(drm_dp_mst_calc_pbn_mode, drm_dp_mst_calc_pbn_mode_cases,
+ dp_mst_calc_pbn_mode_desc);
+
static bool
sideband_msg_req_equal(const struct drm_dp_sideband_msg_req_body *in,
const struct drm_dp_sideband_msg_req_body *out)
@@ -271,7 +300,7 @@ static void drm_test_dp_mst_sideband_msg_req_decode(struct
kunit *test)
}
static struct kunit_case drm_dp_mst_helper_tests[] = {
- KUNIT_CASE(drm_test_dp_mst_calc_pbn_mode),
+ KUNIT_CASE_PARAM(drm_test_dp_mst_calc_pbn_mode,
drm_dp_mst_calc_pbn_mode_gen_params),
KUNIT_CASE(drm_test_dp_mst_sideband_msg_req_decode),
{ }
};
--
2.37.3
[PATCH v3 2/2] drm/tests: Split drm_test_dp_mst_sideband_msg_req_decode into parameterized tests
The drm_test_dp_mst_sideband_msg_req_decode repeats the same test
structure with different parameters. This could be better represented
by parameterized tests, provided by KUnit.
In addition to the parameterization of the tests, the test case for the
client ID was changed: instead of using get_random_bytes to generate
the client ID, the client ID is now hardcoded in the test case. This
doesn't affect the assertively of the tests, as this test case only compare
the data going in with the data going out and it doesn't transform the data
itself in any way.
So, convert drm_test_dp_mst_sideband_msg_req_decode into parameterized
tests and make the tests' allocations and prints completely managed by KUnit.
Signed-off-by: Maíra Canal
---
v1 -> v2:
https://lore.kernel.org/dri-devel/20220925222719.345424-1-mca...@igalia.com/T/#m056610a23a63109484afeafefb5846178c4d36b2
- Mention on the commit message the change on the test case for the client ID
(Michał Winiarski).
v2 -> v3:
https://lore.kernel.org/dri-devel/20220927221206.55930-1-mca...@igalia.com/T/#m2dc961da2d4921566cd0f9a8ed9d2d33a1cf4416
- Mention on the commit message that the "random" usage is not incompatible
with parameterized tests (Michał Winiarski).
---
.../gpu/drm/tests/drm_dp_mst_helper_test.c| 370 --
1 file changed, 243 insertions(+), 127 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
index 12f41881db6b..545beea33e8c 100644
--- a/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_dp_mst_helper_test.c
@@ -5,12 +5,8 @@
* Copyright (c) 2022 Maíra Canal
*/
-#define PREFIX_STR "[drm_dp_mst_helper]"
-
#include
-#include
-
#include
#include
@@ -72,6 +68,217 @@ static void dp_mst_calc_pbn_mode_desc(const struct
drm_dp_mst_calc_pbn_mode_test
KUNIT_ARRAY_PARAM(drm_dp_mst_calc_pbn_mode, drm_dp_mst_calc_pbn_mode_cases,
dp_mst_calc_pbn_mode_desc);
+static u8 data[] = { 0xff, 0x00, 0xdd };
+
+struct drm_dp_mst_sideband_msg_req_test {
+ const char *desc;
+ const struct drm_dp_sideband_msg_req_body in;
+};
+
+static const struct drm_dp_mst_sideband_msg_req_test
drm_dp_mst_sideband_msg_req_cases[] = {
+ {
+ .desc = "DP_ENUM_PATH_RESOURCES with port number",
+ .in = {
+ .req_type = DP_ENUM_PATH_RESOURCES,
+ .u.port_num.port_number = 5,
+ },
+ },
+ {
+ .desc = "DP_POWER_UP_PHY with port number",
+ .in = {
+ .req_type = DP_POWER_UP_PHY,
+ .u.port_num.port_number = 5,
+ },
+ },
+ {
+ .desc = "DP_POWER_DOWN_PHY with port number",
+ .in = {
+ .req_type = DP_POWER_DOWN_PHY,
+ .u.port_num.port_number = 5,
+ },
+ },
+ {
+ .desc = "DP_ALLOCATE_PAYLOAD with SDP stream sinks",
+ .in = {
+ .req_type = DP_ALLOCATE_PAYLOAD,
+ .u.allocate_payload.number_sdp_streams = 3,
+ .u.allocate_payload.sdp_stream_sink = { 1, 2, 3 },
+ },
+ },
+ {
+ .desc = "DP_ALLOCATE_PAYLOAD with port number",
+ .in = {
+ .req_type = DP_ALLOCATE_PAYLOAD,
+ .u.allocate_payload.port_number = 0xf,
+ },
+ },
+ {
+ .desc = "DP_ALLOCATE_PAYLOAD with VCPI",
+ .in = {
+ .req_type = DP_ALLOCATE_PAYLOAD,
+ .u.allocate_payload.vcpi = 0x7f,
+ },
+ },
+ {
+ .desc = "DP_ALLOCATE_PAYLOAD with PBN",
+ .in = {
+ .req_type = DP_ALLOCATE_PAYLOAD,
+ .u.allocate_payload.pbn = U16_MAX,
+ },
+ },
+ {
+ .desc = "DP_QUERY_PAYLOAD with port number",
+ .in = {
+ .req_type = DP_QUERY_PAYLOAD,
+ .u.query_payload.port_number = 0xf,
+ },
+ },
+ {
+ .desc = "DP_QUERY_PAYLOAD with VCPI",
+ .in = {
+ .req_type = DP_QUERY_PAYLOAD,
+ .u.query_payload.vcpi = 0x7f,
+ },
+ },
+ {
+ .desc = "DP_REMOTE_DPCD_READ with port number",
+ .in = {
+ .req_type = DP_REMOTE_DPCD_READ,
+ .u.dpcd_read.port_number = 0xf,
+ },
+ },
+ {
+ .desc = "DP_REMOTE_DPCD_READ with DPCD address",
+ .in = {
+ .req_type = DP_REMOTE_DPCD_READ,
+ .u.dpcd_read.dpcd_address = 0xfedcb,
+ },
+ },
+ {
+
[v6] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280
Flush mechanism for DSPP blocks has changed in sc7280 family, it
allows individual sub blocks to be flushed in coordination with
master flush control.
Representation: master_flush && (PCC_flush | IGC_flush .. etc )
This change adds necessary support for the above design.
Changes in v1:
- Few nits (Doug, Dmitry)
- Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry)
Changes in v2:
- Move the address offset to flush macro (Dmitry)
- Seperate ops for the sub block flush (Dmitry)
Changes in v3:
- Reuse the DPU_DSPP_xx enum instead of a new one (Dmitry)
Changes in v4:
- Use shorter version for unsigned int (Stephen)
Changes in v5:
- Spurious patch please ignore.
Changes in v6:
- Add SOB tag (Doug, Dmitry)
Signed-off-by: Kalyan Thota
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++--
5 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 601d687..4170fbe 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -766,7 +766,7 @@ static void _dpu_crtc_setup_cp_blocks(struct drm_crtc *crtc)
/* stage config flush mask */
ctl->ops.update_pending_flush_dspp(ctl,
- mixer[i].hw_dspp->idx);
+ mixer[i].hw_dspp->idx, DPU_DSPP_PCC);
}
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 27f029f..0eecb2f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -65,7 +65,10 @@
(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
#define CTL_SC7280_MASK \
- (BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) |
BIT(DPU_CTL_VM_CFG))
+ (BIT(DPU_CTL_ACTIVE_CFG) | \
+BIT(DPU_CTL_FETCH_ACTIVE) | \
+BIT(DPU_CTL_VM_CFG) | \
+BIT(DPU_CTL_DSPP_SUB_BLOCK_FLUSH))
#define MERGE_3D_SM8150_MASK (0)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 38aa38a..8148e91 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -161,10 +161,12 @@ enum {
* DSPP sub-blocks
* @DPU_DSPP_PCC Panel color correction block
* @DPU_DSPP_GC Gamma correction block
+ * @DPU_DSPP_IGC Inverse Gamma correction block
*/
enum {
DPU_DSPP_PCC = 0x1,
DPU_DSPP_GC,
+ DPU_DSPP_IGC,
DPU_DSPP_MAX
};
@@ -191,6 +193,7 @@ enum {
* @DPU_CTL_SPLIT_DISPLAY: CTL supports video mode split display
* @DPU_CTL_FETCH_ACTIVE: Active CTL for fetch HW (SSPPs)
* @DPU_CTL_VM_CFG:CTL config to support multiple VMs
+ * @DPU_CTL_DSPP_BLOCK_FLUSH: CTL config to support dspp sub-block flush
* @DPU_CTL_MAX
*/
enum {
@@ -198,6 +201,7 @@ enum {
DPU_CTL_ACTIVE_CFG,
DPU_CTL_FETCH_ACTIVE,
DPU_CTL_VM_CFG,
+ DPU_CTL_DSPP_SUB_BLOCK_FLUSH,
DPU_CTL_MAX
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index a35ecb6..f26f484 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -33,6 +33,7 @@
#define CTL_INTF_FLUSH0x110
#define CTL_INTF_MASTER 0x134
#define CTL_FETCH_PIPE_ACTIVE 0x0FC
+#define CTL_DSPP_n_FLUSH(n) ((0x13C) + ((n - 1) * 4))
#define CTL_MIXER_BORDER_OUTBIT(24)
#define CTL_FLUSH_MASK_CTL BIT(17)
@@ -287,8 +288,9 @@ static void
dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
}
static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
- enum dpu_dspp dspp)
+ enum dpu_dspp dspp, u32 dspp_sub_blk)
{
+
switch (dspp) {
case DSPP_0:
ctx->pending_flush_mask |= BIT(13);
@@ -307,6 +309,31 @@ static void dpu_hw_ctl_update_pending_flush_dspp(struct
dpu_hw_ctl *ctx,
}
}
+static void dpu_hw_ctl_update_pending_flush_dspp_subblocks(
+ struct dpu_hw_ctl *ctx, enum dpu_dspp dspp, u32 dspp_sub_blk)
+{
+ u32 flushbits = 0, active;
+
+ switch (dspp_sub_blk) {
+ case DPU_DSPP_IGC:
+ flushbits = BIT(2);
+ break;
+ case DPU_DSPP_PCC:
+ flushbits = BIT(4);
+ break;
+ case DPU_DSPP_GC:
+ flushbits = BIT(5);
+ break;
+ default:
+ return;
+ }
+
+ active = DPU_REG_READ(&ctx->hw, CTL_DSPP_n_FLUSH(dspp));
Re: [PATCH 14/16] drm/i915/vm_bind: Handle persistent vmas in execbuf3
On Fri, Sep 30, 2022 at 10:47:48AM +0100, Matthew Auld wrote:
On 28/09/2022 07:19, Niranjana Vishwanathapura wrote:
Handle persistent (VM_BIND) mappings during the request submission
in the execbuf3 path.
Signed-off-by: Niranjana Vishwanathapura
Signed-off-by: Andi Shyti
---
.../gpu/drm/i915/gem/i915_gem_execbuffer3.c | 188 +-
1 file changed, 187 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer3.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer3.c
index 92af88bc8deb..1aeeff5e8540 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer3.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer3.c
@@ -19,6 +19,7 @@
#include "i915_gem_vm_bind.h"
#include "i915_trace.h"
+#define __EXEC3_HAS_PINBIT_ULL(33)
#define __EXEC3_ENGINE_PINNED BIT_ULL(32)
#define __EXEC3_INTERNAL_FLAGS (~0ull << 32)
@@ -42,7 +43,9 @@
* execlist. Hence, no support for implicit sync.
*
* The new execbuf3 ioctl only works in VM_BIND mode and the VM_BIND mode only
- * works with execbuf3 ioctl for submission.
+ * works with execbuf3 ioctl for submission. All BOs mapped on that VM (through
+ * VM_BIND call) at the time of execbuf3 call are deemed required for that
+ * submission.
*
* The execbuf3 ioctl directly specifies the batch addresses instead of as
* object handles as in execbuf2 ioctl. The execbuf3 ioctl will also not
@@ -58,6 +61,13 @@
* So, a lot of code supporting execbuf2 ioctl, like relocations, VA evictions,
* vma lookup table, implicit sync, vma active reference tracking etc., are not
* applicable for execbuf3 ioctl.
+ *
+ * During each execbuf submission, request fence is added to all VM_BIND mapped
+ * objects with DMA_RESV_USAGE_BOOKKEEP. The DMA_RESV_USAGE_BOOKKEEP usage will
+ * prevent over sync (See enum dma_resv_usage). Note that DRM_I915_GEM_WAIT and
+ * DRM_I915_GEM_BUSY ioctls do not check for DMA_RESV_USAGE_BOOKKEEP usage and
+ * hence should not be used for end of batch check. Instead, the execbuf3
+ * timeline out fence should be used for end of batch check.
*/
/**
@@ -127,6 +137,23 @@ eb_find_vma(struct i915_address_space *vm, u64 addr)
return i915_gem_vm_bind_lookup_vma(vm, va);
}
+static void eb_scoop_unbound_vma_all(struct i915_address_space *vm)
+{
+ struct i915_vma *vma, *vn;
+
+ /**
+* Move all unbound vmas back into vm_bind_list so that they are
+* revalidated.
+*/
+ spin_lock(&vm->vm_rebind_lock);
+ list_for_each_entry_safe(vma, vn, &vm->vm_rebind_list, vm_rebind_link) {
+ list_del_init(&vma->vm_rebind_link);
+ if (!list_empty(&vma->vm_bind_link))
+ list_move_tail(&vma->vm_bind_link, &vm->vm_bind_list);
+ }
+ spin_unlock(&vm->vm_rebind_lock);
+}
+
static int eb_lookup_vma_all(struct i915_execbuffer *eb)
{
unsigned int i, current_batch = 0;
@@ -141,14 +168,121 @@ static int eb_lookup_vma_all(struct i915_execbuffer *eb)
++current_batch;
}
+ eb_scoop_unbound_vma_all(eb->context->vm);
+
+ return 0;
+}
+
+static int eb_lock_vma_all(struct i915_execbuffer *eb)
+{
+ struct i915_address_space *vm = eb->context->vm;
+ struct i915_vma *vma;
+ int err;
+
+ err = i915_gem_object_lock(eb->context->vm->root_obj, &eb->ww);
+ if (err)
+ return err;
+
+ list_for_each_entry(vma, &vm->non_priv_vm_bind_list,
+ non_priv_vm_bind_link) {
+ err = i915_gem_object_lock(vma->obj, &eb->ww);
+ if (err)
+ return err;
+ }
+
return 0;
}
+static void eb_release_persistent_vma_all(struct i915_execbuffer *eb,
+ bool final)
+{
+ struct i915_address_space *vm = eb->context->vm;
+ struct i915_vma *vma, *vn;
+
+ lockdep_assert_held(&vm->vm_bind_lock);
+
+ if (!(eb->args->flags & __EXEC3_HAS_PIN))
+ return;
+
+ assert_object_held(vm->root_obj);
+
+ list_for_each_entry(vma, &vm->vm_bind_list, vm_bind_link)
+ __i915_vma_unpin(vma);
+
+ eb->args->flags &= ~__EXEC3_HAS_PIN;
+ if (!final)
+ return;
+
+ list_for_each_entry_safe(vma, vn, &vm->vm_bind_list, vm_bind_link)
+ if (i915_vma_verify_bind_complete(vma))
+ list_move_tail(&vma->vm_bind_link, &vm->vm_bound_list);
+}
+
static void eb_release_vma_all(struct i915_execbuffer *eb, bool final)
{
+ eb_release_persistent_vma_all(eb, final);
eb_unpin_engine(eb);
}
+static int eb_reserve_fence_for_persistent_vma_all(struct i915_execbuffer *eb)
+{
+ struct i915_address_space *vm = eb->context->vm;
+ struct i915_vma *vma;
+ int ret;
+
+ ret = dma_resv_reserve_fences(vm->root_obj->base.resv, 1);
+ if (ret)
+ return ret;
+
+ list_for_each_entry(vma,
[RFC PATCH 0/4] Add RGB ttl connection on rockchip phy
The rockchip phy can be convigured in ttl mode. The phy is shared
between lvds, dsi, ttl. The configuration that now I'm able to support
has the display output on some set of pins on standard vop output
and a set of pins using the ttl phy. The solution is not clean as I
would like to have because some register that are used to enable
the TTL, are in the same register area of the dsi controller.
In order to test I must add the following
dsi_dphy: phy@ff2e {
reg = <0x0 0xff2e 0x0 0x1>,
<0x0 0xff45 0x0 0x1>;
...
}
The problem here is the second region I have added is the same of
dsi logic. Only one register is needed by the the phy driver
Michael Trimarchi (4):
phy: add PHY_MODE_TTL
phy: rockchip: Add inno_is_valid_phy_mode
phy: rockchip: Implement TTY phy mode
drm/rockchip: rgb: Add dphy connection to rgb output
drivers/gpu/drm/rockchip/rockchip_rgb.c | 18 +
.../phy/rockchip/phy-rockchip-inno-dsidphy.c | 72 +++
include/linux/phy/phy.h | 3 +-
3 files changed, 92 insertions(+), 1 deletion(-)
--
2.34.1
[RFC PATCH 1/4] phy: add PHY_MODE_TTL
There are combo phys out there that can be switched between doing
dsi, lvds, and ttl. So add a mode definition for it.
Signed-off-by: Michael Trimarchi
---
include/linux/phy/phy.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index b1413757fcc3..87ae8c27ec57 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -42,7 +42,8 @@ enum phy_mode {
PHY_MODE_MIPI_DPHY,
PHY_MODE_SATA,
PHY_MODE_LVDS,
- PHY_MODE_DP
+ PHY_MODE_DP,
+ PHY_MODE_TTL
};
enum phy_media {
--
2.34.1
[RFC PATCH 3/4] phy: rockchip: Implement TTY phy mode
The rockchip phy can be programmed in 3 modes:
- dsi
- lvds
- ttl
For instance in px30 there are two sets of rgb interface pins m0 and m1.
The logic can go outside from the VOP using m0 set or go outside using
the m1 set and the ttl logic enable. There are combination where a set
of pin can be taken from m1 and m0 where all the two path are enabled.
dsi and ttl enable share one register in their register area. Simple
implementation is overlap the area where we want access the register
Signed-off-by: Michael Trimarchi
---
.../phy/rockchip/phy-rockchip-inno-dsidphy.c | 53 +++
1 file changed, 53 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
index 644cf73cfd53..0af50d2e0402 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
@@ -217,6 +217,17 @@ static void phy_update_bits(struct inno_dsidphy *inno,
writel(tmp, inno->phy_base + reg);
}
+static void host_update_bits(struct inno_dsidphy *inno,
+u32 reg, u32 mask, u32 val)
+{
+ unsigned int tmp, orig;
+
+ orig = readl(inno->host_base + reg);
+ tmp = orig & ~mask;
+ tmp |= val & mask;
+ writel(tmp, inno->host_base + reg);
+}
+
static int inno_is_valid_phy_mode(struct inno_dsidphy *inno)
{
switch (inno->mode) {
@@ -224,6 +235,10 @@ static int inno_is_valid_phy_mode(struct inno_dsidphy
*inno)
break;
case PHY_MODE_LVDS:
break;
+ case PHY_MODE_TTL:
+ if (IS_ERR(inno->host_base))
+ return -EINVAL;
+ break;
default:
return -EINVAL;
}
@@ -506,6 +521,32 @@ static void inno_dsidphy_lvds_mode_enable(struct
inno_dsidphy *inno)
LVDS_DATA_LANE2_EN | LVDS_DATA_LANE3_EN);
}
+static void inno_dsidphy_ttl_mode_enable(struct inno_dsidphy *inno)
+{
+ /* Select TTL mode */
+ phy_update_bits(inno, REGISTER_PART_LVDS, 0x03,
+ MODE_ENABLE_MASK, TTL_MODE_ENABLE);
+ /* Reset digital logic */
+ phy_update_bits(inno, REGISTER_PART_LVDS, 0x00,
+ LVDS_DIGITAL_INTERNAL_RESET_MASK,
+ LVDS_DIGITAL_INTERNAL_RESET_ENABLE);
+ udelay(1);
+ phy_update_bits(inno, REGISTER_PART_LVDS, 0x00,
+ LVDS_DIGITAL_INTERNAL_RESET_MASK,
+ LVDS_DIGITAL_INTERNAL_RESET_DISABLE);
+ /* Enable digital logic */
+ phy_update_bits(inno, REGISTER_PART_LVDS, 0x01,
+ LVDS_DIGITAL_INTERNAL_ENABLE_MASK,
+ LVDS_DIGITAL_INTERNAL_ENABLE);
+ /* Enable analog driver */
+ phy_update_bits(inno, REGISTER_PART_LVDS, 0x0b,
+ LVDS_LANE_EN_MASK, LVDS_CLK_LANE_EN |
+ LVDS_DATA_LANE0_EN | LVDS_DATA_LANE1_EN |
+ LVDS_DATA_LANE2_EN | LVDS_DATA_LANE3_EN);
+ /* Enable for clk lane in TTL mode */
+ host_update_bits(inno, DSI_PHY_RSTZ, PHY_ENABLECLK, PHY_ENABLECLK);
+}
+
static int inno_dsidphy_power_on(struct phy *phy)
{
struct inno_dsidphy *inno = phy_get_drvdata(phy);
@@ -533,6 +574,9 @@ static int inno_dsidphy_power_on(struct phy *phy)
case PHY_MODE_LVDS:
inno_dsidphy_lvds_mode_enable(inno);
break;
+ case PHY_MODE_TTL:
+ inno_dsidphy_ttl_mode_enable(inno);
+ break;
default:
return -EINVAL;
}
@@ -561,6 +605,10 @@ static int inno_dsidphy_power_off(struct phy *phy)
LVDS_PLL_POWER_MASK | LVDS_BANDGAP_POWER_MASK,
LVDS_PLL_POWER_OFF | LVDS_BANDGAP_POWER_DOWN);
+ /* Disable for clk lane in TTL mode */
+ if (!IS_ERR(inno->host_base))
+ host_update_bits(inno, DSI_PHY_RSTZ, PHY_ENABLECLK, 0);
+
pm_runtime_put(inno->dev);
clk_disable_unprepare(inno->ref_clk);
clk_disable_unprepare(inno->pclk_phy);
@@ -576,6 +624,7 @@ static int inno_dsidphy_set_mode(struct phy *phy, enum
phy_mode mode,
switch (mode) {
case PHY_MODE_MIPI_DPHY:
case PHY_MODE_LVDS:
+ case PHY_MODE_TTL:
inno->mode = mode;
break;
default:
@@ -630,6 +679,10 @@ static int inno_dsidphy_probe(struct platform_device *pdev)
if (IS_ERR(inno->phy_base))
return PTR_ERR(inno->phy_base);
+ inno->host_base = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(inno->host_base))
+ dev_warn(dev, "TTL mode is not supported\n");
+
inno->ref_clk = devm_clk_get(dev, "ref");
if (IS_ERR(inno->ref_clk)) {
ret = PTR_ERR(inno->ref_clk);
--
2.34.1
[RFC PATCH 2/4] phy: rockchip: Add inno_is_valid_phy_mode
The function is used to avoid to enable clock on the hardware
if the mode requested is invalid
Signed-off-by: Michael Trimarchi
---
.../phy/rockchip/phy-rockchip-inno-dsidphy.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
index 630e01b5c19b..644cf73cfd53 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c
@@ -217,6 +217,20 @@ static void phy_update_bits(struct inno_dsidphy *inno,
writel(tmp, inno->phy_base + reg);
}
+static int inno_is_valid_phy_mode(struct inno_dsidphy *inno)
+{
+ switch (inno->mode) {
+ case PHY_MODE_MIPI_DPHY:
+ break;
+ case PHY_MODE_LVDS:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+};
+
static unsigned long inno_dsidphy_pll_calc_rate(struct inno_dsidphy *inno,
unsigned long rate)
{
@@ -495,6 +509,11 @@ static void inno_dsidphy_lvds_mode_enable(struct
inno_dsidphy *inno)
static int inno_dsidphy_power_on(struct phy *phy)
{
struct inno_dsidphy *inno = phy_get_drvdata(phy);
+ int ret = 0;
+
+ ret = inno_is_valid_phy_mode(inno);
+ if (ret)
+ return ret;
clk_prepare_enable(inno->pclk_phy);
clk_prepare_enable(inno->ref_clk);
--
2.34.1
[RFC PATCH 4/4] drm/rockchip: rgb: Add dphy connection to rgb output
Dispite the commit 1f0f015151727, the rgb output has an option
to allow to sent the output pin using the dsi/lvds/ttl logic.
The only way to do and stay on the same design is let the
rockchip_rgb block to grab the handle if it is present and
enable it. The present of this handle depends on dts configuration
I have a full working example with an hardware with mixed lines
on direct logic and using the phy, with the follow dts example:
panel: panel {
compatible = "panel-dpi";
...
panel-timing {
clock-frequency = <3000>;
...
};
port {
panel_rgb_in: endpoint {
remote-endpoint = <&vopb_out_rgb>;
};
};
};
&vopb_out {
vopb_out_rgb: endpoint@2 {
reg = <2>;
remote-endpoint = <&panel_rgb_in>;
};
};
&vopb {
status = "okay";
pinctrl-names = "default", "sleep";
pinctrl-0 = <&lcdc_rgb_pins>;
pinctrl-1 = <&lcdc_sleep_pins>;
phys = <&dsi_dphy>;
phy-names = "dphy";
};
Signed-off-by: Michael Trimarchi
---
drivers/gpu/drm/rockchip/rockchip_rgb.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c
b/drivers/gpu/drm/rockchip/rockchip_rgb.c
index 75eb7cca3d82..c725774a0f40 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
@@ -30,6 +31,7 @@ struct rockchip_rgb {
struct drm_bridge *bridge;
struct drm_encoder encoder;
struct drm_connector connector;
+ struct phy *dphy;
int output_mode;
};
@@ -168,6 +170,22 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
goto err_free_connector;
}
+ /* PHY */
+ rgb->dphy = devm_phy_get(dev, "dphy");
+ if (!IS_ERR(rgb->dphy)) {
+ ret = phy_init(rgb->dphy);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ret = phy_set_mode(rgb->dphy, PHY_MODE_TTL);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ret = phy_power_on(rgb->dphy);
+ if (ret)
+ return ERR_PTR(ret);
+ }
+
return rgb;
err_free_connector:
--
2.34.1
