[PATCH RFC 0/4] Support for Simulated Panels

[PATCH RFC 0/4] Support for Simulated Panels

[Jessica Zhang]

This series introduces a simulated MIPI DSI panel.

Currently, the only way to validate DSI connectors is with a physical
panel. Since obtaining physical panels for all possible DSI configurations
is logistically infeasible, introduce a way for DSI drivers to simulate a
panel.

This will be helpful in catching DSI misconfiguration bugs and catching
performance issues for high FPS panels that might not be easily
obtainable.

For now, the simulated panel driver only supports setting customized
modes via the panel_simlation.mode modparam. Eventually, we would like
to add more customizations (such as configuring DSC, dual DSI, etc.).

---
Jessica Zhang (4):
      drm/panel: add driver for simulated panel
      drm/dsi: Add API to register simulated DSI panel
      drm/panel: Introduce simulated panel bridge API
      drm/msm/dsi: Add simulated panel support

 drivers/gpu/drm/bridge/panel.c           |  24 +++++
 drivers/gpu/drm/drm_mipi_dsi.c           |  30 +++++++
 drivers/gpu/drm/drm_panel.c              |  33 +++++++
 drivers/gpu/drm/msm/dsi/dsi.c            |   4 +
 drivers/gpu/drm/msm/dsi/dsi_host.c       |   9 +-
 drivers/gpu/drm/msm/dsi/dsi_manager.c    |   8 +-
 drivers/gpu/drm/panel/Kconfig            |   9 ++
 drivers/gpu/drm/panel/Makefile           |   1 +
 drivers/gpu/drm/panel/panel-simulation.c | 147 +++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h                 |   1 +
 include/drm/drm_mipi_dsi.h               |   1 +
 include/drm/drm_panel.h                  |   1 +
 12 files changed, 266 insertions(+), 2 deletions(-)
---
base-commit: 9ba3471618f1ab8df2f2689a34a505a72e05760a
change-id: 20240102-jz-test-sim-panel-71c14a56716e

Best regards,
-- 
Jessica Zhang <quic_jesszhan@quicinc.com>

[PATCH RFC 1/4] drm/panel: add driver for simulated panel

[Jessica Zhang]

Add a driver for simulating panels. This module also supports a mode
parameter for users to specify a custom mode. If no custom mode is set,
it will fall back to a custom, hard-coded mode.

Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/panel/Kconfig            |   9 ++
 drivers/gpu/drm/panel/Makefile           |   1 +
 drivers/gpu/drm/panel/panel-simulation.c | 147 +++++++++++++++++++++++++++++++
 3 files changed, 157 insertions(+)

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 99e14dc212ecb..d711ec170c586 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -107,6 +107,15 @@ config DRM_PANEL_SIMPLE
 	  that it can be automatically turned off when the panel goes into a
 	  low power state.
 
+config DRM_PANEL_SIMULATION
+	tristate "support for simulation panels"
+	depends on DRM_MIPI_DSI
+	help
+	  DRM panel driver for simulated DSI panels. Enabling this config will
+	  cause the physical panel driver to not be attached to the DT panel
+	  node. After the kernel boots, users can load the module and specify a
+	  custom mode using the driver modparams.
+
 config DRM_PANEL_EDP
 	tristate "support for simple Embedded DisplayPort panels"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d10c3de51c6db..5bc55357714ad 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_DRM_PANEL_BOE_TV101WUM_NL6) += panel-boe-tv101wum-nl6.o
 obj-$(CONFIG_DRM_PANEL_DSI_CM) += panel-dsi-cm.o
 obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
+obj-$(CONFIG_DRM_PANEL_SIMULATION) += panel-simulation.o
 obj-$(CONFIG_DRM_PANEL_EDP) += panel-edp.o
 obj-$(CONFIG_DRM_PANEL_EBBG_FT8719) += panel-ebbg-ft8719.o
 obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o
diff --git a/drivers/gpu/drm/panel/panel-simulation.c b/drivers/gpu/drm/panel/panel-simulation.c
new file mode 100644
index 0000000000000..081c03bea188d
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-simulation.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_panel.h>
+
+static char sim_panel_mode[PATH_MAX];
+
+module_param_string(mode, sim_panel_mode, sizeof(sim_panel_mode), 0644);
+MODULE_PARM_DESC(mode, "Sim panel mode");
+
+struct panel_simulation {
+	struct drm_panel base;
+	struct platform_device *platform;
+} *sim_panel;
+
+static struct drm_display_mode panel_simulation_mode = {
+	.clock = 345830,
+	.hdisplay = 1080,
+	.hsync_start = 1175,
+	.hsync_end = 1176,
+	.htotal = 1216,
+	.vdisplay = 2340,
+	.vsync_start = 2365,
+	.vsync_end = 2366,
+	.vtotal = 2370,
+	.width_mm = 0,
+	.height_mm = 0,
+	.type = DRM_MODE_TYPE_DRIVER,
+};
+
+static int panel_simulation_parse_mode(void)
+{
+	int count;
+	struct drm_display_mode user_mode = { 0 };
+	unsigned int vrefresh;
+
+	if (sim_panel_mode[0] == '\0')
+		return 0;
+
+	count = sscanf(sim_panel_mode, "%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu-%u",
+			     &user_mode.hdisplay, &user_mode.hsync_start,
+			     &user_mode.hsync_end, &user_mode.htotal,
+			     &user_mode.vdisplay, &user_mode.vsync_start,
+			     &user_mode.vsync_end, &user_mode.vtotal, &vrefresh);
+
+	if (count != 9)
+		return -EINVAL;
+
+	user_mode.clock = user_mode.htotal * user_mode.vtotal * vrefresh / 1000;
+	memcpy(&panel_simulation_mode, &user_mode, sizeof(struct drm_display_mode));
+
+	return 0;
+}
+
+static int panel_simulation_get_modes(struct drm_panel *panel,
+				    struct drm_connector *connector)
+{
+	struct drm_display_mode *mode;
+	int ret;
+
+	ret = panel_simulation_parse_mode();
+
+	mode = drm_mode_duplicate(connector->dev, &panel_simulation_mode);
+	if (!mode)
+		return -ENOMEM;
+
+	drm_mode_set_name(mode);
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs panel_simulation_funcs = {
+	.get_modes = panel_simulation_get_modes,
+};
+
+static int panel_simulation_probe(struct mipi_dsi_device *dsi)
+{
+	struct panel_simulation *panel;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
+	if (!panel)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, panel);
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS;
+
+	drm_panel_init(&panel->base, dev, &panel_simulation_funcs, DRM_MODE_CONNECTOR_DSI);
+	drm_panel_add(&panel->base);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret)
+		drm_panel_remove(&panel->base);
+
+	return ret;
+}
+
+static void panel_simulation_remove(struct mipi_dsi_device *dsi)
+{
+	struct panel_simulation *panel = mipi_dsi_get_drvdata(dsi);
+	int err;
+
+	err = mipi_dsi_detach(dsi);
+	if (err < 0)
+		dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
+
+	drm_panel_remove(&panel->base);
+	drm_panel_disable(&panel->base);
+	drm_panel_unprepare(&panel->base);
+}
+
+static void panel_simulation_shutdown(struct mipi_dsi_device *dsi)
+{
+	struct panel_simulation *panel = dev_get_drvdata(&dsi->dev);
+
+	drm_panel_disable(&panel->base);
+	drm_panel_unprepare(&panel->base);
+}
+
+static struct mipi_dsi_driver panel_simulation_driver = {
+	.driver = {
+		.name = "panel_simulation",
+	},
+	.probe = panel_simulation_probe,
+	.remove = panel_simulation_remove,
+	.shutdown = panel_simulation_shutdown,
+};
+module_mipi_dsi_driver(panel_simulation_driver);
+
+MODULE_AUTHOR("Jessica Zhang <quic_jesszhan@quicinc.com>");
+MODULE_DESCRIPTION("DRM Driver for Simulated Panels");
+MODULE_LICENSE("GPL");

-- 
2.43.0

[PATCH RFC 2/4] drm/dsi: Add API to register simulated DSI panel

[Jessica Zhang]

Add new APIs to register a simulated panel.

For drivers that want to support a simulated panel, they must call
mipi_dsi_host_register_sim_panel().

Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 30 ++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h     |  1 +
 2 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 843a6dbda93a0..6996014990979 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -153,6 +153,24 @@ static int mipi_dsi_device_add(struct mipi_dsi_device *dsi)
 	return device_add(&dsi->dev);
 }
 
+#if IS_ENABLED(CONFIG_DRM_PANEL_SIMULATION)
+static struct mipi_dsi_device *mipi_dsi_device_add_sim_panel(struct mipi_dsi_host *host)
+{
+	struct mipi_dsi_device_info info = { };
+
+	info.channel = 0;
+	info.node = NULL;
+	strscpy(info.type, "panel_simulation", sizeof(info.type));
+
+	return mipi_dsi_device_register_full(host, &info);
+}
+#else
+static struct mipi_dsi_device *mipi_dsi_device_add_sim_panel(struct mipi_dsi_host *host)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
 #if IS_ENABLED(CONFIG_OF)
 static struct mipi_dsi_device *
 of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
@@ -324,6 +342,18 @@ struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node)
 }
 EXPORT_SYMBOL(of_find_mipi_dsi_host_by_node);
 
+int mipi_dsi_host_register_sim_panel(struct mipi_dsi_host *host)
+{
+	mipi_dsi_device_add_sim_panel(host);
+
+	mutex_lock(&host_lock);
+	list_add_tail(&host->list, &host_list);
+	mutex_unlock(&host_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(mipi_dsi_host_register_sim_panel);
+
 int mipi_dsi_host_register(struct mipi_dsi_host *host)
 {
 	struct device_node *node;
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index c0aec0d4d664e..4ca44b7b3efdb 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -107,6 +107,7 @@ struct mipi_dsi_host {
 	struct list_head list;
 };
 
+int mipi_dsi_host_register_sim_panel(struct mipi_dsi_host *host);
 int mipi_dsi_host_register(struct mipi_dsi_host *host);
 void mipi_dsi_host_unregister(struct mipi_dsi_host *host);
 struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node);

-- 
2.43.0

[PATCH RFC 3/4] drm/panel: Introduce simulated panel bridge API

[Jessica Zhang]

Add separate bridge and drm_panel API for getting the simulated panel.

Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/bridge/panel.c | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/drm_panel.c    | 33 +++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h       |  1 +
 include/drm/drm_panel.h        |  1 +
 4 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index e48823a4f1ede..87a83f4ce68a1 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -499,6 +499,30 @@ struct drm_connector *drm_panel_bridge_connector(struct drm_bridge *bridge)
 }
 EXPORT_SYMBOL(drm_panel_bridge_connector);
 
+/**
+ * drm_get_sim_panel_bridge - return the simulated panel bridge
+ * @dev: device to tie the bridge lifetime to
+ *
+ * This function will return a bridge for the simulated panel.
+ *
+ * Returns a pointer to the bridge if successful, or an error pointer
+ * otherwise.
+ */
+struct drm_bridge *drm_get_sim_panel_bridge(struct device *dev)
+{
+	struct drm_bridge *bridge;
+	struct drm_panel *panel;
+
+	panel = drm_find_sim_panel();
+	if (IS_ERR(panel))
+		return ERR_PTR(-EPROBE_DEFER);
+
+	bridge = devm_drm_panel_bridge_add(dev, panel);
+
+	return bridge;
+}
+EXPORT_SYMBOL(drm_get_sim_panel_bridge);
+
 #ifdef CONFIG_OF
 /**
  * devm_drm_of_get_bridge - Return next bridge in the chain
diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c
index e814020bbcd3b..062541505fa74 100644
--- a/drivers/gpu/drm/drm_panel.c
+++ b/drivers/gpu/drm/drm_panel.c
@@ -290,6 +290,39 @@ int drm_panel_get_modes(struct drm_panel *panel,
 }
 EXPORT_SYMBOL(drm_panel_get_modes);
 
+/**
+ * drm_find_sim_panel - look up the simulated panel
+ *
+ * Searches for the simulated panel in the panel list.
+ *
+ * Return: A pointer to the simulated panel or an ERR_PTR() if the simulated
+ * panel was not found in the panel list.
+ *
+ * Possible error codes returned by this function:
+ * - EPROBE_DEFER: the panel device has not been probed yet, and the caller
+ *   should retry later
+*/
+struct drm_panel *drm_find_sim_panel(void)
+{
+	struct drm_panel *panel;
+
+	mutex_lock(&panel_lock);
+
+	list_for_each_entry(panel, &panel_list, list) {
+		bool is_sim_panel = !strncmp(panel->dev->driver->name,
+				"panel_simulation",
+				strlen("panel_simulation"));
+		if (is_sim_panel) {
+			mutex_unlock(&panel_lock);
+			return panel;
+		}
+	}
+
+	mutex_unlock(&panel_lock);
+	return ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL(drm_find_sim_panel);
+
 #ifdef CONFIG_OF
 /**
  * of_drm_find_panel - look up a panel using a device tree node
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index e39da5807ba71..941f1f825e2c6 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -927,6 +927,7 @@ static inline int drm_panel_bridge_set_orientation(struct drm_connector *connect
 }
 #endif
 
+struct drm_bridge *drm_get_sim_panel_bridge(struct device *dev);
 #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL_BRIDGE)
 struct drm_bridge *devm_drm_of_get_bridge(struct device *dev, struct device_node *node,
 					  u32 port, u32 endpoint);
diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h
index 10015891b056f..c3a5944c35a91 100644
--- a/include/drm/drm_panel.h
+++ b/include/drm/drm_panel.h
@@ -283,6 +283,7 @@ int drm_panel_enable(struct drm_panel *panel);
 int drm_panel_disable(struct drm_panel *panel);
 
 int drm_panel_get_modes(struct drm_panel *panel, struct drm_connector *connector);
+struct drm_panel *drm_find_sim_panel(void);
 
 #if defined(CONFIG_OF) && defined(CONFIG_DRM_PANEL)
 struct drm_panel *of_drm_find_panel(const struct device_node *np);

-- 
2.43.0

[PATCH RFC 4/4] drm/msm/dsi: Add simulated panel support

[Jessica Zhang]

Introduce the sim_panel_enabled module parameter.

When set, this parameter will force DSI to select the simulated panel
instead of the physical panel.

Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/dsi.c         | 4 ++++
 drivers/gpu/drm/msm/dsi/dsi_host.c    | 9 ++++++++-
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 +++++++-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index c6bd7bf15605c..daea84f5e3c0c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -5,6 +5,10 @@
 
 #include "dsi.h"
 
+bool sim_panel_enabled;
+MODULE_PARM_DESC(sim_panel_enabled, "Use simulated panel");
+module_param(sim_panel_enabled, bool, 0444);
+
 bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
 {
 	unsigned long host_flags = msm_dsi_host_get_mode_flags(msm_dsi->host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index deeecdfd6c4e4..fa0cab09fff71 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -35,6 +35,8 @@
 
 #define DSI_RESET_TOGGLE_DELAY_MS 20
 
+extern bool sim_panel_enabled;
+
 static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc_config *dsc);
 
 static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
@@ -2009,7 +2011,12 @@ int msm_dsi_host_register(struct mipi_dsi_host *host)
 	if (!msm_host->registered) {
 		host->dev = &msm_host->pdev->dev;
 		host->ops = &dsi_host_ops;
-		ret = mipi_dsi_host_register(host);
+
+		if (sim_panel_enabled)
+			ret = mipi_dsi_host_register_sim_panel(host);
+		else
+			ret = mipi_dsi_host_register(host);
+
 		if (ret)
 			return ret;
 
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 896f369fdd535..e33e6be7309f2 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -30,6 +30,8 @@ struct msm_dsi_manager {
 
 static struct msm_dsi_manager msm_dsim_glb;
 
+extern bool sim_panel_enabled;
+
 #define IS_BONDED_DSI()		(msm_dsim_glb.is_bonded_dsi)
 #define IS_SYNC_NEEDED()	(msm_dsim_glb.is_sync_needed)
 #define IS_MASTER_DSI_LINK(id)	(msm_dsim_glb.master_dsi_link_id == id)
@@ -507,7 +509,11 @@ int msm_dsi_manager_ext_bridge_init(u8 id)
 	int ret;
 
 	int_bridge = msm_dsi->bridge;
-	ext_bridge = devm_drm_of_get_bridge(&msm_dsi->pdev->dev,
+
+	if (sim_panel_enabled)
+		ext_bridge = drm_get_sim_panel_bridge(&msm_dsi->pdev->dev);
+	else
+		ext_bridge = devm_drm_of_get_bridge(&msm_dsi->pdev->dev,
 					    msm_dsi->pdev->dev.of_node, 1, 0);
 	if (IS_ERR(ext_bridge))
 		return PTR_ERR(ext_bridge);

-- 
2.43.0

Re: [PATCH RFC 0/4] Support for Simulated Panels

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

Hi,

On Tue, Jan 16, 2024 at 02:22:03PM -0800, Jessica Zhang wrote:
> This series introduces a simulated MIPI DSI panel.
> 
> Currently, the only way to validate DSI connectors is with a physical
> panel. Since obtaining physical panels for all possible DSI configurations
> is logistically infeasible, introduce a way for DSI drivers to simulate a
> panel.
> 
> This will be helpful in catching DSI misconfiguration bugs and catching
> performance issues for high FPS panels that might not be easily
> obtainable.
> 
> For now, the simulated panel driver only supports setting customized
> modes via the panel_simlation.mode modparam. Eventually, we would like
> to add more customizations (such as configuring DSC, dual DSI, etc.).

I think that it's more complicated than it needs to be.

Why do we need to support (and switch to) both the actual and
"simulated" panel?

Wouldn't it be simpler if we had a vkms-like panel that we could either
configure from DT or from debugfs that would just be registered the
usual way and would be the only panel we register?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC 0/4] Support for Simulated Panels

[Jani Nikula]

On Wed, 17 Jan 2024, Maxime Ripard <mripard@kernel.org> wrote:
> Hi,
>
> On Tue, Jan 16, 2024 at 02:22:03PM -0800, Jessica Zhang wrote:
>> This series introduces a simulated MIPI DSI panel.
>> 
>> Currently, the only way to validate DSI connectors is with a physical
>> panel. Since obtaining physical panels for all possible DSI configurations
>> is logistically infeasible, introduce a way for DSI drivers to simulate a
>> panel.
>> 
>> This will be helpful in catching DSI misconfiguration bugs and catching
>> performance issues for high FPS panels that might not be easily
>> obtainable.
>> 
>> For now, the simulated panel driver only supports setting customized
>> modes via the panel_simlation.mode modparam. Eventually, we would like
>> to add more customizations (such as configuring DSC, dual DSI, etc.).
>
> I think that it's more complicated than it needs to be.

Both too complicated and not complicated enough! :p

> Why do we need to support (and switch to) both the actual and
> "simulated" panel?
>
> Wouldn't it be simpler if we had a vkms-like panel that we could either
> configure from DT or from debugfs that would just be registered the
> usual way and would be the only panel we register?

I get the idea of trying to test DSI code without panels, and looking at
the goals above, I think your vkms suggestion is going to fall short of
those goals.

However, my gut feeling is that creating a simulated panel to catch DSI
misconfiguration etc. is going to be insanely complicated, and this
series doesn't even scratch the surface.

I guess my questions are, what's the scope here really, are those goals
realistic, does more code already exist beyond this skeleton?

BR,
Jani.



-- 
Jani Nikula, Intel

Re: [PATCH RFC 0/4] Support for Simulated Panels

[Abhinav Kumar]

Hi Jani and Maxime

On 1/17/2024 2:16 AM, Jani Nikula wrote:
> On Wed, 17 Jan 2024, Maxime Ripard <mripard@kernel.org> wrote:
>> Hi,
>>
>> On Tue, Jan 16, 2024 at 02:22:03PM -0800, Jessica Zhang wrote:
>>> This series introduces a simulated MIPI DSI panel.
>>>
>>> Currently, the only way to validate DSI connectors is with a physical
>>> panel. Since obtaining physical panels for all possible DSI configurations
>>> is logistically infeasible, introduce a way for DSI drivers to simulate a
>>> panel.
>>>
>>> This will be helpful in catching DSI misconfiguration bugs and catching
>>> performance issues for high FPS panels that might not be easily
>>> obtainable.
>>>
>>> For now, the simulated panel driver only supports setting customized
>>> modes via the panel_simlation.mode modparam. Eventually, we would like
>>> to add more customizations (such as configuring DSC, dual DSI, etc.).
>>
>> I think that it's more complicated than it needs to be.
> 
> Both too complicated and not complicated enough! :p
> 

The end goal is to have a framework to be able to validate the display 
pipeline with MIPI panels of any resolution , DSC/non-DSC, different 
MIPI flags etc.

Historically, QC has been having an in-house framework to validate the 
panels in a simulated way as its logistically not possible to procure 
every panel from every vendor. This has been working pretty well but its 
not upstream yet. So we would like to work with the community to work on 
a model which works for everyone and this RFC was initiated with that in 
mind.

There is simulation infrastructure in place in upstream for HDMI/DP in 
the form of chamelium based testing in IGT but no such fwk exists for 
DSI displays.

Different MIPI panels and resolutions test out not only the DSI 
controller but the entire display pipeline as based on resolution, 
compression and MIPI mode flags different parts of the pipeline can get 
exercised.

>> Why do we need to support (and switch to) both the actual and
>> "simulated" panel?
>>

As per my discussion on IRC with the panel/bridge maintainers and DT 
maintainers, a simulation panel does not qualify for its own devicetree 
as its not a real hardware so we needed to come up with a way to have a 
module which can be attached to the encoder without its own bindings and 
devicetree. Thats what led to this RFC.

>> Wouldn't it be simpler if we had a vkms-like panel that we could either
>> configure from DT or from debugfs that would just be registered the
>> usual way and would be the only panel we register?
> 

No, we need to have validate actual hardware pipeline with the simulated 
panel. With vkms, actual display pipeline will not be validated. With 
incorrect display pipeline misconfigurations arising from different 
panel combinations, this can easily be caught with any existing IGT CRC 
testing. In addition, all performance related bugs can also be easily 
caught by simulating high resolution displays.

> I get the idea of trying to test DSI code without panels, and looking at
> the goals above, I think your vkms suggestion is going to fall short of
> those goals.
> 
> However, my gut feeling is that creating a simulated panel to catch DSI
> misconfiguration etc. is going to be insanely complicated, and this
> series doesn't even scratch the surface.
> 
> I guess my questions are, what's the scope here really, are those goals
> realistic, does more code already exist beyond this skeleton?
> 


This series is only a starting RFC to be able to validate any display 
mode. This would have to be extended to be able to customize different 
pieces of the panel. Lets talk about the customizable pieces:

1) Display resolution with timings (drm_display_mode)
2) Compression/non-compression
3) Command mode/Video mode
4) MIPI mode flags
5) DCS commands for panel enable/disable and other panel sequences
6) Power-up/Power-down sequence for the panel

Without a physical panel, yes its hard to validate if anything is wrong 
with (4) OR (5), the display might not come up at all visually. But from 
our experience, thats only a small portion and the real benefit of this 
framework will actually be from the validation failures we will catch 
from (1) to (4).

This RFC only provides a way to customize (1) at the moment as we wanted 
to get some feedback from the community about the best way which will 
work for everyone to customize all these parameters.

We are willing to expand this series based on the generic way we agree 
on to customize other params.

Yes, debugfs is an option too. But typically MIPI displays need some 
parameters configured to attach the panel to the encoder. So perhaps we 
can boot the simulation panel with a default resolution passed through 
command line and then across a modeset switch (1) to (4).

Thanks

Abhinav
> BR,
> Jani.
> 
> 
> 

Re: [PATCH RFC 4/4] drm/msm/dsi: Add simulated panel support

[Dmitry Baryshkov]

On Wed, 17 Jan 2024 at 00:22, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>
> Introduce the sim_panel_enabled module parameter.
>
> When set, this parameter will force DSI to select the simulated panel
> instead of the physical panel.
>
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dsi/dsi.c         | 4 ++++
>  drivers/gpu/drm/msm/dsi/dsi_host.c    | 9 ++++++++-
>  drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 +++++++-
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> index c6bd7bf15605c..daea84f5e3c0c 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> @@ -5,6 +5,10 @@
>
>  #include "dsi.h"
>
> +bool sim_panel_enabled;
> +MODULE_PARM_DESC(sim_panel_enabled, "Use simulated panel");
> +module_param(sim_panel_enabled, bool, 0444);
> +
>  bool msm_dsi_is_cmd_mode(struct msm_dsi *msm_dsi)
>  {
>         unsigned long host_flags = msm_dsi_host_get_mode_flags(msm_dsi->host);
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index deeecdfd6c4e4..fa0cab09fff71 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -35,6 +35,8 @@
>
>  #define DSI_RESET_TOGGLE_DELAY_MS 20
>
> +extern bool sim_panel_enabled;
> +
>  static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc_config *dsc);
>
>  static int dsi_get_version(const void __iomem *base, u32 *major, u32 *minor)
> @@ -2009,7 +2011,12 @@ int msm_dsi_host_register(struct mipi_dsi_host *host)
>         if (!msm_host->registered) {
>                 host->dev = &msm_host->pdev->dev;
>                 host->ops = &dsi_host_ops;
> -               ret = mipi_dsi_host_register(host);
> +
> +               if (sim_panel_enabled)
> +                       ret = mipi_dsi_host_register_sim_panel(host);
> +               else
> +                       ret = mipi_dsi_host_register(host);
> +
>                 if (ret)
>                         return ret;
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index 896f369fdd535..e33e6be7309f2 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -30,6 +30,8 @@ struct msm_dsi_manager {
>
>  static struct msm_dsi_manager msm_dsim_glb;
>
> +extern bool sim_panel_enabled;
> +
>  #define IS_BONDED_DSI()                (msm_dsim_glb.is_bonded_dsi)
>  #define IS_SYNC_NEEDED()       (msm_dsim_glb.is_sync_needed)
>  #define IS_MASTER_DSI_LINK(id) (msm_dsim_glb.master_dsi_link_id == id)
> @@ -507,7 +509,11 @@ int msm_dsi_manager_ext_bridge_init(u8 id)
>         int ret;
>
>         int_bridge = msm_dsi->bridge;
> -       ext_bridge = devm_drm_of_get_bridge(&msm_dsi->pdev->dev,
> +
> +       if (sim_panel_enabled)
> +               ext_bridge = drm_get_sim_panel_bridge(&msm_dsi->pdev->dev);
> +       else
> +               ext_bridge = devm_drm_of_get_bridge(&msm_dsi->pdev->dev,
>                                             msm_dsi->pdev->dev.of_node, 1, 0);

I think that this is definitely an imperfect way to go. We should not
be modifying the DSI host drivers to be able to use the simulation
panel.
Could you please push this to drm_of_find_panel_or_bridge() ?

>         if (IS_ERR(ext_bridge))
>                 return PTR_ERR(ext_bridge);
>
> --
> 2.43.0
>


-- 
With best wishes
Dmitry

Re: [PATCH RFC 1/4] drm/panel: add driver for simulated panel

[Dmitry Baryshkov]

On Wed, 17 Jan 2024 at 00:31, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>
> Add a driver for simulating panels. This module also supports a mode
> parameter for users to specify a custom mode. If no custom mode is set,
> it will fall back to a custom, hard-coded mode.
>
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>  drivers/gpu/drm/panel/Kconfig            |   9 ++
>  drivers/gpu/drm/panel/Makefile           |   1 +
>  drivers/gpu/drm/panel/panel-simulation.c | 147 +++++++++++++++++++++++++++++++
>  3 files changed, 157 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 99e14dc212ecb..d711ec170c586 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -107,6 +107,15 @@ config DRM_PANEL_SIMPLE
>           that it can be automatically turned off when the panel goes into a
>           low power state.
>
> +config DRM_PANEL_SIMULATION
> +       tristate "support for simulation panels"
> +       depends on DRM_MIPI_DSI
> +       help
> +         DRM panel driver for simulated DSI panels. Enabling this config will
> +         cause the physical panel driver to not be attached to the DT panel
> +         node. After the kernel boots, users can load the module and specify a
> +         custom mode using the driver modparams.
> +
>  config DRM_PANEL_EDP
>         tristate "support for simple Embedded DisplayPort panels"
>         depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index d10c3de51c6db..5bc55357714ad 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_DRM_PANEL_BOE_TV101WUM_NL6) += panel-boe-tv101wum-nl6.o
>  obj-$(CONFIG_DRM_PANEL_DSI_CM) += panel-dsi-cm.o
>  obj-$(CONFIG_DRM_PANEL_LVDS) += panel-lvds.o
>  obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
> +obj-$(CONFIG_DRM_PANEL_SIMULATION) += panel-simulation.o
>  obj-$(CONFIG_DRM_PANEL_EDP) += panel-edp.o
>  obj-$(CONFIG_DRM_PANEL_EBBG_FT8719) += panel-ebbg-ft8719.o
>  obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o
> diff --git a/drivers/gpu/drm/panel/panel-simulation.c b/drivers/gpu/drm/panel/panel-simulation.c
> new file mode 100644
> index 0000000000000..081c03bea188d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-simulation.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +
> +static char sim_panel_mode[PATH_MAX];
> +
> +module_param_string(mode, sim_panel_mode, sizeof(sim_panel_mode), 0644);
> +MODULE_PARM_DESC(mode, "Sim panel mode");
> +
> +struct panel_simulation {
> +       struct drm_panel base;
> +       struct platform_device *platform;
> +} *sim_panel;
> +
> +static struct drm_display_mode panel_simulation_mode = {
> +       .clock = 345830,
> +       .hdisplay = 1080,
> +       .hsync_start = 1175,
> +       .hsync_end = 1176,
> +       .htotal = 1216,
> +       .vdisplay = 2340,
> +       .vsync_start = 2365,
> +       .vsync_end = 2366,
> +       .vtotal = 2370,
> +       .width_mm = 0,
> +       .height_mm = 0,
> +       .type = DRM_MODE_TYPE_DRIVER,
> +};
> +
> +static int panel_simulation_parse_mode(void)
> +{
> +       int count;
> +       struct drm_display_mode user_mode = { 0 };
> +       unsigned int vrefresh;
> +
> +       if (sim_panel_mode[0] == '\0')
> +               return 0;
> +
> +       count = sscanf(sim_panel_mode, "%hu,%hu,%hu,%hu,%hu,%hu,%hu,%hu-%u",
> +                            &user_mode.hdisplay, &user_mode.hsync_start,
> +                            &user_mode.hsync_end, &user_mode.htotal,
> +                            &user_mode.vdisplay, &user_mode.vsync_start,
> +                            &user_mode.vsync_end, &user_mode.vtotal, &vrefresh);
> +
> +       if (count != 9)
> +               return -EINVAL;
> +
> +       user_mode.clock = user_mode.htotal * user_mode.vtotal * vrefresh / 1000;

Does this compile / work on 32-bit platforms?

> +       memcpy(&panel_simulation_mode, &user_mode, sizeof(struct drm_display_mode));

We probably should set dpi through the modparam and calculate width_mm
/ height_mm here

> +
> +       return 0;
> +}
> +
> +static int panel_simulation_get_modes(struct drm_panel *panel,
> +                                   struct drm_connector *connector)
> +{
> +       struct drm_display_mode *mode;
> +       int ret;
> +
> +       ret = panel_simulation_parse_mode();
> +
> +       mode = drm_mode_duplicate(connector->dev, &panel_simulation_mode);
> +       if (!mode)
> +               return -ENOMEM;
> +
> +       drm_mode_set_name(mode);
> +       mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +       connector->display_info.width_mm = mode->width_mm;
> +       connector->display_info.height_mm = mode->height_mm;
> +       drm_mode_probed_add(connector, mode);


drm_connector_helper_get_modes_fixed() ?

> +
> +       return 1;
> +}
> +
> +static const struct drm_panel_funcs panel_simulation_funcs = {
> +       .get_modes = panel_simulation_get_modes,
> +};
> +
> +static int panel_simulation_probe(struct mipi_dsi_device *dsi)
> +{
> +       struct panel_simulation *panel;
> +       struct device *dev = &dsi->dev;
> +       int ret;
> +
> +       panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL);
> +       if (!panel)
> +               return -ENOMEM;
> +
> +       mipi_dsi_set_drvdata(dsi, panel);
> +
> +       dsi->lanes = 4;
> +       dsi->format = MIPI_DSI_FMT_RGB888;
> +       dsi->mode_flags = MIPI_DSI_MODE_LPM | MIPI_DSI_CLOCK_NON_CONTINUOUS;

I think these might need to be configurable too. Maybe in a follow up patch.

> +
> +       drm_panel_init(&panel->base, dev, &panel_simulation_funcs, DRM_MODE_CONNECTOR_DSI);
> +       drm_panel_add(&panel->base);
> +
> +       ret = mipi_dsi_attach(dsi);
> +       if (ret)
> +               drm_panel_remove(&panel->base);
> +
> +       return ret;
> +}
> +
> +static void panel_simulation_remove(struct mipi_dsi_device *dsi)
> +{
> +       struct panel_simulation *panel = mipi_dsi_get_drvdata(dsi);
> +       int err;
> +
> +       err = mipi_dsi_detach(dsi);
> +       if (err < 0)
> +               dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", err);
> +
> +       drm_panel_remove(&panel->base);
> +       drm_panel_disable(&panel->base);
> +       drm_panel_unprepare(&panel->base);
> +}
> +
> +static void panel_simulation_shutdown(struct mipi_dsi_device *dsi)
> +{
> +       struct panel_simulation *panel = dev_get_drvdata(&dsi->dev);
> +
> +       drm_panel_disable(&panel->base);
> +       drm_panel_unprepare(&panel->base);
> +}
> +
> +static struct mipi_dsi_driver panel_simulation_driver = {
> +       .driver = {
> +               .name = "panel_simulation",
> +       },
> +       .probe = panel_simulation_probe,
> +       .remove = panel_simulation_remove,
> +       .shutdown = panel_simulation_shutdown,
> +};
> +module_mipi_dsi_driver(panel_simulation_driver);
> +
> +MODULE_AUTHOR("Jessica Zhang <quic_jesszhan@quicinc.com>");
> +MODULE_DESCRIPTION("DRM Driver for Simulated Panels");
> +MODULE_LICENSE("GPL");
>
> --
> 2.43.0
>


-- 
With best wishes
Dmitry

Re: Re: [PATCH RFC 0/4] Support for Simulated Panels

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 6151 bytes --]

On Wed, Jan 17, 2024 at 09:36:20AM -0800, Abhinav Kumar wrote:
> Hi Jani and Maxime
> 
> On 1/17/2024 2:16 AM, Jani Nikula wrote:
> > On Wed, 17 Jan 2024, Maxime Ripard <mripard@kernel.org> wrote:
> > > Hi,
> > > 
> > > On Tue, Jan 16, 2024 at 02:22:03PM -0800, Jessica Zhang wrote:
> > > > This series introduces a simulated MIPI DSI panel.
> > > > 
> > > > Currently, the only way to validate DSI connectors is with a physical
> > > > panel. Since obtaining physical panels for all possible DSI configurations
> > > > is logistically infeasible, introduce a way for DSI drivers to simulate a
> > > > panel.
> > > > 
> > > > This will be helpful in catching DSI misconfiguration bugs and catching
> > > > performance issues for high FPS panels that might not be easily
> > > > obtainable.
> > > > 
> > > > For now, the simulated panel driver only supports setting customized
> > > > modes via the panel_simlation.mode modparam. Eventually, we would like
> > > > to add more customizations (such as configuring DSC, dual DSI, etc.).
> > > 
> > > I think that it's more complicated than it needs to be.
> > 
> > Both too complicated and not complicated enough! :p
>
> The end goal is to have a framework to be able to validate the display
> pipeline with MIPI panels of any resolution , DSC/non-DSC, different MIPI
> flags etc.
> 
> Historically, QC has been having an in-house framework to validate the
> panels in a simulated way as its logistically not possible to procure every
> panel from every vendor. This has been working pretty well but its not
> upstream yet. So we would like to work with the community to work on a model
> which works for everyone and this RFC was initiated with that in mind.

I think the goal was pretty clear. My point was more that there's no
reason it should be driver specific, and having a second path for it
doesn't really exert the actual panel path in the driver. I think a
separate driver would be better.

> There is simulation infrastructure in place in upstream for HDMI/DP in the
> form of chamelium based testing in IGT but no such fwk exists for DSI
> displays.
> 
> Different MIPI panels and resolutions test out not only the DSI controller
> but the entire display pipeline as based on resolution, compression and MIPI
> mode flags different parts of the pipeline can get exercised.
> 
> > > Why do we need to support (and switch to) both the actual and
> > > "simulated" panel?
> > > 
> 
> As per my discussion on IRC with the panel/bridge maintainers and DT
> maintainers, a simulation panel does not qualify for its own devicetree as
> its not a real hardware so we needed to come up with a way to have a module
> which can be attached to the encoder without its own bindings and
> devicetree. Thats what led to this RFC.

I still think it's worth trying, there's plenty of virtual drivers in
the DT already. But even then, DT policies shouldn't dictate general
framework design decisions: we have other ways to probe panels than
using the DT (by loading overlays, registering devices by hand, etc.). I
still think it would be a good idea to try though.

> > > Wouldn't it be simpler if we had a vkms-like panel that we could either
> > > configure from DT or from debugfs that would just be registered the
> > > usual way and would be the only panel we register?
> > 
> 
> No, we need to have validate actual hardware pipeline with the simulated
> panel. With vkms, actual display pipeline will not be validated. With
> incorrect display pipeline misconfigurations arising from different panel
> combinations, this can easily be caught with any existing IGT CRC testing.
> In addition, all performance related bugs can also be easily caught by
> simulating high resolution displays.

That's not what I meant. What I meant was that something like a
user-configurable, generic, panel driver would be a good idea. Just like
vkms (with the debugfs patches) is for a full blown KMS device.

> > I get the idea of trying to test DSI code without panels, and looking at
> > the goals above, I think your vkms suggestion is going to fall short of
> > those goals.
> > 
> > However, my gut feeling is that creating a simulated panel to catch DSI
> > misconfiguration etc. is going to be insanely complicated, and this
> > series doesn't even scratch the surface.
> > 
> > I guess my questions are, what's the scope here really, are those goals
> > realistic, does more code already exist beyond this skeleton?
> > 
> 
> 
> This series is only a starting RFC to be able to validate any display mode.
> This would have to be extended to be able to customize different pieces of
> the panel. Lets talk about the customizable pieces:
> 
> 1) Display resolution with timings (drm_display_mode)
> 2) Compression/non-compression
> 3) Command mode/Video mode
> 4) MIPI mode flags
> 5) DCS commands for panel enable/disable and other panel sequences
> 6) Power-up/Power-down sequence for the panel
> 
> Without a physical panel, yes its hard to validate if anything is wrong with
> (4) OR (5), the display might not come up at all visually. But from our
> experience, thats only a small portion and the real benefit of this
> framework will actually be from the validation failures we will catch from
> (1) to (4).
> 
> This RFC only provides a way to customize (1) at the moment as we wanted to
> get some feedback from the community about the best way which will work for
> everyone to customize all these parameters.
> 
> We are willing to expand this series based on the generic way we agree on to
> customize other params.
> 
> Yes, debugfs is an option too. But typically MIPI displays need some
> parameters configured to attach the panel to the encoder. So perhaps we can
> boot the simulation panel with a default resolution passed through command
> line and then across a modeset switch (1) to (4).

I think Jani's feeling was that it was going to be super complicated
fairly fast so supporting more features would definitely help to get an
idea of where this is going.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC 0/4] Support for Simulated Panels

[Abhinav Kumar]

<adding device tree maintainers to comment>

Hi Maxime

On 1/26/2024 4:45 AM, Maxime Ripard wrote:
> On Wed, Jan 17, 2024 at 09:36:20AM -0800, Abhinav Kumar wrote:
>> Hi Jani and Maxime
>>
>> On 1/17/2024 2:16 AM, Jani Nikula wrote:
>>> On Wed, 17 Jan 2024, Maxime Ripard <mripard@kernel.org> wrote:
>>>> Hi,
>>>>
>>>> On Tue, Jan 16, 2024 at 02:22:03PM -0800, Jessica Zhang wrote:
>>>>> This series introduces a simulated MIPI DSI panel.
>>>>>
>>>>> Currently, the only way to validate DSI connectors is with a physical
>>>>> panel. Since obtaining physical panels for all possible DSI configurations
>>>>> is logistically infeasible, introduce a way for DSI drivers to simulate a
>>>>> panel.
>>>>>
>>>>> This will be helpful in catching DSI misconfiguration bugs and catching
>>>>> performance issues for high FPS panels that might not be easily
>>>>> obtainable.
>>>>>
>>>>> For now, the simulated panel driver only supports setting customized
>>>>> modes via the panel_simlation.mode modparam. Eventually, we would like
>>>>> to add more customizations (such as configuring DSC, dual DSI, etc.).
>>>>
>>>> I think that it's more complicated than it needs to be.
>>>
>>> Both too complicated and not complicated enough! :p
>>
>> The end goal is to have a framework to be able to validate the display
>> pipeline with MIPI panels of any resolution , DSC/non-DSC, different MIPI
>> flags etc.
>>
>> Historically, QC has been having an in-house framework to validate the
>> panels in a simulated way as its logistically not possible to procure every
>> panel from every vendor. This has been working pretty well but its not
>> upstream yet. So we would like to work with the community to work on a model
>> which works for everyone and this RFC was initiated with that in mind.
> 
> I think the goal was pretty clear. My point was more that there's no
> reason it should be driver specific, and having a second path for it
> doesn't really exert the actual panel path in the driver. I think a
> separate driver would be better.
> 

We can make this generic. That would be great actually. One option could 
be to move the modparam we have within the msm to the drm_of.c so that 
drm_of_find_panel_or_bridge shall return the sim panel if the modparam 
is passed to select a sim panel.

So if we make this a compile time decision whether to use real panel or 
sim panel and just enable the appropriate config, we dont need the 
modparam and we can implement some policy in the drm_of to first check 
if sim panel is available and if not try the real panel then everything 
will just happen under-the-hood. But we thought that a modparam based 
switching might be convenient if users dont want to recompile the code 
to switch but will need to compile both the panels.

>> There is simulation infrastructure in place in upstream for HDMI/DP in the
>> form of chamelium based testing in IGT but no such fwk exists for DSI
>> displays.
>>
>> Different MIPI panels and resolutions test out not only the DSI controller
>> but the entire display pipeline as based on resolution, compression and MIPI
>> mode flags different parts of the pipeline can get exercised.
>>
>>>> Why do we need to support (and switch to) both the actual and
>>>> "simulated" panel?
>>>>
>>
>> As per my discussion on IRC with the panel/bridge maintainers and DT
>> maintainers, a simulation panel does not qualify for its own devicetree as
>> its not a real hardware so we needed to come up with a way to have a module
>> which can be attached to the encoder without its own bindings and
>> devicetree. Thats what led to this RFC.
> 
> I still think it's worth trying, there's plenty of virtual drivers in
> the DT already. But even then, DT policies shouldn't dictate general
> framework design decisions: we have other ways to probe panels than
> using the DT (by loading overlays, registering devices by hand, etc.). I
> still think it would be a good idea to try though.
> 

DT option would be great if accepted and will nicely solve the 
scalability issue of this as it desperately needs one.

I have absolutely no concerns and would be glad if it will be accepted.

Can the DT maintainers please comment if having a device tree for a 
simulation panel would work OR be considered because of the scalability 
of the number of panels which can be tried as Maxime wrote.

>>>> Wouldn't it be simpler if we had a vkms-like panel that we could either
>>>> configure from DT or from debugfs that would just be registered the
>>>> usual way and would be the only panel we register?
>>>
>>
>> No, we need to have validate actual hardware pipeline with the simulated
>> panel. With vkms, actual display pipeline will not be validated. With
>> incorrect display pipeline misconfigurations arising from different panel
>> combinations, this can easily be caught with any existing IGT CRC testing.
>> In addition, all performance related bugs can also be easily caught by
>> simulating high resolution displays.
> 
> That's not what I meant. What I meant was that something like a
> user-configurable, generic, panel driver would be a good idea. Just like
> vkms (with the debugfs patches) is for a full blown KMS device.
> 

Let me respond for both this question and the one below from you/Jani.

Certainly having user-configurable information is a goal here. The 
end-goal is to make everything there in the existing panels such as 
below like I wrote:

1) Display resolution with timings (drm_display_mode)
2) Compression/non-compression
3) Command mode/Video mode
4) MIPI mode flags
5) DCS commands for panel enable/disable and other panel sequences
6) Power-up/Power-down sequence for the panel

But, we also have to see what all is feasible today from the DRM fwk 
standpoint. There are some limitations about what is boot-time 
configurable using bootparams and what is runtime configurable (across a 
modeset) using debugfs.

1) Today, everything part of struct mipi_dsi_device needs to be 
available at boot time from what I can see as we need that while calling 
mipi_dsi_attach(). So for that we went with boot-params.

2) For the list of modes, we can move this to a debugfs like 
"populate_modes" which the client using a sim panel can call before 
picking a mode and triggering a commit.

But we need to have some default mode and configuration.

This is where I am not totally sure of. On HDMI/DP sinks, we usually go 
with a default of 640x480 as that one is guaranteed to be supported 
across sinks.

For MIPI displays, we will have to agree on some default configuration then.

So, we can certainly add debugfs to make the runtime params but we need 
to start with some default during boot-up and move the others to debugfs.

With vkms, can you pls point us to the debugfs patches you are referring 
to? With the current vkms, very little is available which is debugfs 
configurable (overlay, writeback and cursor support).

Ofcourse, all these concerns go away if DT option gets accepted.

>>> I get the idea of trying to test DSI code without panels, and looking at
>>> the goals above, I think your vkms suggestion is going to fall short of
>>> those goals.
>>>
>>> However, my gut feeling is that creating a simulated panel to catch DSI
>>> misconfiguration etc. is going to be insanely complicated, and this
>>> series doesn't even scratch the surface.
>>>
>>> I guess my questions are, what's the scope here really, are those goals
>>> realistic, does more code already exist beyond this skeleton?
>>>
>>
>>
>> This series is only a starting RFC to be able to validate any display mode.
>> This would have to be extended to be able to customize different pieces of
>> the panel. Lets talk about the customizable pieces:
>>
>> 1) Display resolution with timings (drm_display_mode)
>> 2) Compression/non-compression
>> 3) Command mode/Video mode
>> 4) MIPI mode flags
>> 5) DCS commands for panel enable/disable and other panel sequences
>> 6) Power-up/Power-down sequence for the panel
>>
>> Without a physical panel, yes its hard to validate if anything is wrong with
>> (4) OR (5), the display might not come up at all visually. But from our
>> experience, thats only a small portion and the real benefit of this
>> framework will actually be from the validation failures we will catch from
>> (1) to (4).
>>
>> This RFC only provides a way to customize (1) at the moment as we wanted to
>> get some feedback from the community about the best way which will work for
>> everyone to customize all these parameters.
>>
>> We are willing to expand this series based on the generic way we agree on to
>> customize other params.
>>
>> Yes, debugfs is an option too. But typically MIPI displays need some
>> parameters configured to attach the panel to the encoder. So perhaps we can
>> boot the simulation panel with a default resolution passed through command
>> line and then across a modeset switch (1) to (4).
> 
> I think Jani's feeling was that it was going to be super complicated
> fairly fast so supporting more features would definitely help to get an
> idea of where this is going.
> 
> Maxime

Re: [PATCH RFC 0/4] Support for Simulated Panels

[Daniel Vetter]

On Mon, Jan 29, 2024 at 11:05:12AM -0800, Abhinav Kumar wrote:
> <adding device tree maintainers to comment>
> 
> Hi Maxime
> 
> On 1/26/2024 4:45 AM, Maxime Ripard wrote:
> > On Wed, Jan 17, 2024 at 09:36:20AM -0800, Abhinav Kumar wrote:
> > > Hi Jani and Maxime
> > > 
> > > On 1/17/2024 2:16 AM, Jani Nikula wrote:
> > > > On Wed, 17 Jan 2024, Maxime Ripard <mripard@kernel.org> wrote:
> > > > > Hi,
> > > > > 
> > > > > On Tue, Jan 16, 2024 at 02:22:03PM -0800, Jessica Zhang wrote:
> > > > > > This series introduces a simulated MIPI DSI panel.
> > > > > > 
> > > > > > Currently, the only way to validate DSI connectors is with a physical
> > > > > > panel. Since obtaining physical panels for all possible DSI configurations
> > > > > > is logistically infeasible, introduce a way for DSI drivers to simulate a
> > > > > > panel.
> > > > > > 
> > > > > > This will be helpful in catching DSI misconfiguration bugs and catching
> > > > > > performance issues for high FPS panels that might not be easily
> > > > > > obtainable.
> > > > > > 
> > > > > > For now, the simulated panel driver only supports setting customized
> > > > > > modes via the panel_simlation.mode modparam. Eventually, we would like
> > > > > > to add more customizations (such as configuring DSC, dual DSI, etc.).
> > > > > 
> > > > > I think that it's more complicated than it needs to be.
> > > > 
> > > > Both too complicated and not complicated enough! :p
> > > 
> > > The end goal is to have a framework to be able to validate the display
> > > pipeline with MIPI panels of any resolution , DSC/non-DSC, different MIPI
> > > flags etc.
> > > 
> > > Historically, QC has been having an in-house framework to validate the
> > > panels in a simulated way as its logistically not possible to procure every
> > > panel from every vendor. This has been working pretty well but its not
> > > upstream yet. So we would like to work with the community to work on a model
> > > which works for everyone and this RFC was initiated with that in mind.
> > 
> > I think the goal was pretty clear. My point was more that there's no
> > reason it should be driver specific, and having a second path for it
> > doesn't really exert the actual panel path in the driver. I think a
> > separate driver would be better.
> > 
> 
> We can make this generic. That would be great actually. One option could be
> to move the modparam we have within the msm to the drm_of.c so that
> drm_of_find_panel_or_bridge shall return the sim panel if the modparam is
> passed to select a sim panel.

Yeah I think gluing this into drm_of_find_panel_or_bridge() would be
great, should help get other drivers on board and also help with
encouraging drivers to use panels/bridges correctly.

> So if we make this a compile time decision whether to use real panel or sim
> panel and just enable the appropriate config, we dont need the modparam and
> we can implement some policy in the drm_of to first check if sim panel is
> available and if not try the real panel then everything will just happen
> under-the-hood. But we thought that a modparam based switching might be
> convenient if users dont want to recompile the code to switch but will need
> to compile both the panels.

I think you can get to this point at runtime too:

- add a debugfs interface to your drm-sim-panel.ko driver. or maybe dt
  overlay or whatever is most convenient to configure the panel to be the
  sim one and not the real one

- load that drm-sim-panel.ko module first and configure it

- load the real driver

- some magic/heuristics/whatever in drm_of_find_panel_or_bridge to make
  sure things work.

Yes this breaks all built-in configs, but I also think trying to configure
a sim panel with modparams is going to be a lost cause, they're really
tricky. See also my comment below.

> > > There is simulation infrastructure in place in upstream for HDMI/DP in the
> > > form of chamelium based testing in IGT but no such fwk exists for DSI
> > > displays.
> > > 
> > > Different MIPI panels and resolutions test out not only the DSI controller
> > > but the entire display pipeline as based on resolution, compression and MIPI
> > > mode flags different parts of the pipeline can get exercised.
> > > 
> > > > > Why do we need to support (and switch to) both the actual and
> > > > > "simulated" panel?
> > > > > 
> > > 
> > > As per my discussion on IRC with the panel/bridge maintainers and DT
> > > maintainers, a simulation panel does not qualify for its own devicetree as
> > > its not a real hardware so we needed to come up with a way to have a module
> > > which can be attached to the encoder without its own bindings and
> > > devicetree. Thats what led to this RFC.
> > 
> > I still think it's worth trying, there's plenty of virtual drivers in
> > the DT already. But even then, DT policies shouldn't dictate general
> > framework design decisions: we have other ways to probe panels than
> > using the DT (by loading overlays, registering devices by hand, etc.). I
> > still think it would be a good idea to try though.
> > 
> 
> DT option would be great if accepted and will nicely solve the scalability
> issue of this as it desperately needs one.
> 
> I have absolutely no concerns and would be glad if it will be accepted.
> 
> Can the DT maintainers please comment if having a device tree for a
> simulation panel would work OR be considered because of the scalability of
> the number of panels which can be tried as Maxime wrote.
> 
> > > > > Wouldn't it be simpler if we had a vkms-like panel that we could either
> > > > > configure from DT or from debugfs that would just be registered the
> > > > > usual way and would be the only panel we register?
> > > > 
> > > 
> > > No, we need to have validate actual hardware pipeline with the simulated
> > > panel. With vkms, actual display pipeline will not be validated. With
> > > incorrect display pipeline misconfigurations arising from different panel
> > > combinations, this can easily be caught with any existing IGT CRC testing.
> > > In addition, all performance related bugs can also be easily caught by
> > > simulating high resolution displays.
> > 
> > That's not what I meant. What I meant was that something like a
> > user-configurable, generic, panel driver would be a good idea. Just like
> > vkms (with the debugfs patches) is for a full blown KMS device.
> > 
> 
> Let me respond for both this question and the one below from you/Jani.
> 
> Certainly having user-configurable information is a goal here. The end-goal
> is to make everything there in the existing panels such as below like I
> wrote:
> 
> 1) Display resolution with timings (drm_display_mode)
> 2) Compression/non-compression
> 3) Command mode/Video mode
> 4) MIPI mode flags
> 5) DCS commands for panel enable/disable and other panel sequences
> 6) Power-up/Power-down sequence for the panel
> 
> But, we also have to see what all is feasible today from the DRM fwk
> standpoint. There are some limitations about what is boot-time configurable
> using bootparams and what is runtime configurable (across a modeset) using
> debugfs.
> 
> 1) Today, everything part of struct mipi_dsi_device needs to be available at
> boot time from what I can see as we need that while calling
> mipi_dsi_attach(). So for that we went with boot-params.
> 
> 2) For the list of modes, we can move this to a debugfs like
> "populate_modes" which the client using a sim panel can call before picking
> a mode and triggering a commit.
> 
> But we need to have some default mode and configuration.

Uh, at the risk of sounding a bit like I'm just chasing the latest
buzzwords, but this sounds like something that's screaming for ebpf. Which
is also the plan we discussed for allowing vkms to simulate more complex
hardware eventually. Same design really:

- use configfs

- all the static stuff that needs to be fixed at panel registration time
  is going to be configfs files. That gives you a _lot_ more flexibility
  than trying to shoehoern everything into modparam. We've started that
  way for vkms, good for a first tech demo, not even close to good enough
  for a real testing/sim driver.

- all the runtime hooks that control the very specific DSI flows would be
  in ebpf. For common cases maybe you can implement some of the callbacks
  entire in the sim driver in C and configure it using configfs, but for
  anything really complex it's probably going to be ebpf.

  And there's just no way you can load ebpf over a modparam, so at that
  point we absolutely do have to have configfs (iirc you can load/attach
  ebpf to files in a fairly flexible way, so that should be doable with
  configfs - it's definitely the plan for vkms atomic_check constraints
  and stuff like that).

> This is where I am not totally sure of. On HDMI/DP sinks, we usually go with
> a default of 640x480 as that one is guaranteed to be supported across sinks.
> 
> For MIPI displays, we will have to agree on some default configuration then.
> 
> So, we can certainly add debugfs to make the runtime params but we need to
> start with some default during boot-up and move the others to debugfs.
> 
> With vkms, can you pls point us to the debugfs patches you are referring to?
> With the current vkms, very little is available which is debugfs
> configurable (overlay, writeback and cursor support).

It should actually be configfs. Unfortunately those patches haven't landed
(yet). I think this is the latest version, not sure why it's not moved
further:

https://lore.kernel.org/dri-devel/20230829053201.423261-1-brpol@chromium.org/

Cheers, Sima

> Ofcourse, all these concerns go away if DT option gets accepted.
> 
> > > > I get the idea of trying to test DSI code without panels, and looking at
> > > > the goals above, I think your vkms suggestion is going to fall short of
> > > > those goals.
> > > > 
> > > > However, my gut feeling is that creating a simulated panel to catch DSI
> > > > misconfiguration etc. is going to be insanely complicated, and this
> > > > series doesn't even scratch the surface.
> > > > 
> > > > I guess my questions are, what's the scope here really, are those goals
> > > > realistic, does more code already exist beyond this skeleton?
> > > > 
> > > 
> > > 
> > > This series is only a starting RFC to be able to validate any display mode.
> > > This would have to be extended to be able to customize different pieces of
> > > the panel. Lets talk about the customizable pieces:
> > > 
> > > 1) Display resolution with timings (drm_display_mode)
> > > 2) Compression/non-compression
> > > 3) Command mode/Video mode
> > > 4) MIPI mode flags
> > > 5) DCS commands for panel enable/disable and other panel sequences
> > > 6) Power-up/Power-down sequence for the panel
> > > 
> > > Without a physical panel, yes its hard to validate if anything is wrong with
> > > (4) OR (5), the display might not come up at all visually. But from our
> > > experience, thats only a small portion and the real benefit of this
> > > framework will actually be from the validation failures we will catch from
> > > (1) to (4).
> > > 
> > > This RFC only provides a way to customize (1) at the moment as we wanted to
> > > get some feedback from the community about the best way which will work for
> > > everyone to customize all these parameters.
> > > 
> > > We are willing to expand this series based on the generic way we agree on to
> > > customize other params.
> > > 
> > > Yes, debugfs is an option too. But typically MIPI displays need some
> > > parameters configured to attach the panel to the encoder. So perhaps we can
> > > boot the simulation panel with a default resolution passed through command
> > > line and then across a modeset switch (1) to (4).
> > 
> > I think Jani's feeling was that it was going to be super complicated
> > fairly fast so supporting more features would definitely help to get an
> > idea of where this is going.
> > 
> > Maxime

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: Re: [PATCH RFC 0/4] Support for Simulated Panels

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 7918 bytes --]

On Mon, Jan 29, 2024 at 11:05:12AM -0800, Abhinav Kumar wrote:
> <adding device tree maintainers to comment>
> 
> Hi Maxime
> 
> On 1/26/2024 4:45 AM, Maxime Ripard wrote:
> > On Wed, Jan 17, 2024 at 09:36:20AM -0800, Abhinav Kumar wrote:
> > > Hi Jani and Maxime
> > > 
> > > On 1/17/2024 2:16 AM, Jani Nikula wrote:
> > > > On Wed, 17 Jan 2024, Maxime Ripard <mripard@kernel.org> wrote:
> > > > > Hi,
> > > > > 
> > > > > On Tue, Jan 16, 2024 at 02:22:03PM -0800, Jessica Zhang wrote:
> > > > > > This series introduces a simulated MIPI DSI panel.
> > > > > > 
> > > > > > Currently, the only way to validate DSI connectors is with a physical
> > > > > > panel. Since obtaining physical panels for all possible DSI configurations
> > > > > > is logistically infeasible, introduce a way for DSI drivers to simulate a
> > > > > > panel.
> > > > > > 
> > > > > > This will be helpful in catching DSI misconfiguration bugs and catching
> > > > > > performance issues for high FPS panels that might not be easily
> > > > > > obtainable.
> > > > > > 
> > > > > > For now, the simulated panel driver only supports setting customized
> > > > > > modes via the panel_simlation.mode modparam. Eventually, we would like
> > > > > > to add more customizations (such as configuring DSC, dual DSI, etc.).
> > > > > 
> > > > > I think that it's more complicated than it needs to be.
> > > > 
> > > > Both too complicated and not complicated enough! :p
> > > 
> > > The end goal is to have a framework to be able to validate the display
> > > pipeline with MIPI panels of any resolution , DSC/non-DSC, different MIPI
> > > flags etc.
> > > 
> > > Historically, QC has been having an in-house framework to validate the
> > > panels in a simulated way as its logistically not possible to procure every
> > > panel from every vendor. This has been working pretty well but its not
> > > upstream yet. So we would like to work with the community to work on a model
> > > which works for everyone and this RFC was initiated with that in mind.
> > 
> > I think the goal was pretty clear. My point was more that there's no
> > reason it should be driver specific, and having a second path for it
> > doesn't really exert the actual panel path in the driver. I think a
> > separate driver would be better.
> > 
> 
> We can make this generic. That would be great actually. One option could be
> to move the modparam we have within the msm to the drm_of.c so that
> drm_of_find_panel_or_bridge shall return the sim panel if the modparam is
> passed to select a sim panel.
> 
> So if we make this a compile time decision whether to use real panel or sim
> panel and just enable the appropriate config, we dont need the modparam and
> we can implement some policy in the drm_of to first check if sim panel is
> available and if not try the real panel then everything will just happen
> under-the-hood. But we thought that a modparam based switching might be
> convenient if users dont want to recompile the code to switch but will need
> to compile both the panels.

I agree that a module parameter to select the simulated panel looks like
the best option compared to a compile time option.

> > > There is simulation infrastructure in place in upstream for HDMI/DP in the
> > > form of chamelium based testing in IGT but no such fwk exists for DSI
> > > displays.
> > > 
> > > Different MIPI panels and resolutions test out not only the DSI controller
> > > but the entire display pipeline as based on resolution, compression and MIPI
> > > mode flags different parts of the pipeline can get exercised.
> > > 
> > > > > Why do we need to support (and switch to) both the actual and
> > > > > "simulated" panel?
> > > > > 
> > > 
> > > As per my discussion on IRC with the panel/bridge maintainers and DT
> > > maintainers, a simulation panel does not qualify for its own devicetree as
> > > its not a real hardware so we needed to come up with a way to have a module
> > > which can be attached to the encoder without its own bindings and
> > > devicetree. Thats what led to this RFC.
> > 
> > I still think it's worth trying, there's plenty of virtual drivers in
> > the DT already. But even then, DT policies shouldn't dictate general
> > framework design decisions: we have other ways to probe panels than
> > using the DT (by loading overlays, registering devices by hand, etc.). I
> > still think it would be a good idea to try though.
> > 
> 
> DT option would be great if accepted and will nicely solve the scalability
> issue of this as it desperately needs one.
> 
> I have absolutely no concerns and would be glad if it will be accepted.
> 
> Can the DT maintainers please comment if having a device tree for a
> simulation panel would work OR be considered because of the scalability of
> the number of panels which can be tried as Maxime wrote.
> 
> > > > > Wouldn't it be simpler if we had a vkms-like panel that we could either
> > > > > configure from DT or from debugfs that would just be registered the
> > > > > usual way and would be the only panel we register?
> > > > 
> > > 
> > > No, we need to have validate actual hardware pipeline with the simulated
> > > panel. With vkms, actual display pipeline will not be validated. With
> > > incorrect display pipeline misconfigurations arising from different panel
> > > combinations, this can easily be caught with any existing IGT CRC testing.
> > > In addition, all performance related bugs can also be easily caught by
> > > simulating high resolution displays.
> > 
> > That's not what I meant. What I meant was that something like a
> > user-configurable, generic, panel driver would be a good idea. Just like
> > vkms (with the debugfs patches) is for a full blown KMS device.
> > 
> 
> Let me respond for both this question and the one below from you/Jani.
> 
> Certainly having user-configurable information is a goal here. The end-goal
> is to make everything there in the existing panels such as below like I
> wrote:
> 
> 1) Display resolution with timings (drm_display_mode)
> 2) Compression/non-compression
> 3) Command mode/Video mode
> 4) MIPI mode flags
> 5) DCS commands for panel enable/disable and other panel sequences
> 6) Power-up/Power-down sequence for the panel
> 
> But, we also have to see what all is feasible today from the DRM fwk
> standpoint. There are some limitations about what is boot-time configurable
> using bootparams and what is runtime configurable (across a modeset) using
> debugfs.
>
> 1) Today, everything part of struct mipi_dsi_device needs to be available at
> boot time from what I can see as we need that while calling
> mipi_dsi_attach(). So for that we went with boot-params.
> 
> 2) For the list of modes, we can move this to a debugfs like
> "populate_modes" which the client using a sim panel can call before picking
> a mode and triggering a commit.
> 
> But we need to have some default mode and configuration.
> 
> This is where I am not totally sure of. On HDMI/DP sinks, we usually go with
> a default of 640x480 as that one is guaranteed to be supported across sinks.
> 
> For MIPI displays, we will have to agree on some default configuration then.
> 
> So, we can certainly add debugfs to make the runtime params but we need to
> start with some default during boot-up and move the others to debugfs.
> 
> With vkms, can you pls point us to the debugfs patches you are referring to?
> With the current vkms, very little is available which is debugfs
> configurable (overlay, writeback and cursor support).
> 
> Ofcourse, all these concerns go away if DT option gets accepted.

Not entirely, no. You won't be able to express the command sequences
properly through the DT for example, so we would need some kind of
interface like that anyway.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Re: [PATCH RFC 0/4] Support for Simulated Panels

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2647 bytes --]

On Tue, Jan 30, 2024 at 09:53:13AM +0100, Daniel Vetter wrote:
> > > > > > Wouldn't it be simpler if we had a vkms-like panel that we could either
> > > > > > configure from DT or from debugfs that would just be registered the
> > > > > > usual way and would be the only panel we register?
> > > > > 
> > > > 
> > > > No, we need to have validate actual hardware pipeline with the simulated
> > > > panel. With vkms, actual display pipeline will not be validated. With
> > > > incorrect display pipeline misconfigurations arising from different panel
> > > > combinations, this can easily be caught with any existing IGT CRC testing.
> > > > In addition, all performance related bugs can also be easily caught by
> > > > simulating high resolution displays.
> > > 
> > > That's not what I meant. What I meant was that something like a
> > > user-configurable, generic, panel driver would be a good idea. Just like
> > > vkms (with the debugfs patches) is for a full blown KMS device.
> > > 
> > 
> > Let me respond for both this question and the one below from you/Jani.
> > 
> > Certainly having user-configurable information is a goal here. The end-goal
> > is to make everything there in the existing panels such as below like I
> > wrote:
> > 
> > 1) Display resolution with timings (drm_display_mode)
> > 2) Compression/non-compression
> > 3) Command mode/Video mode
> > 4) MIPI mode flags
> > 5) DCS commands for panel enable/disable and other panel sequences
> > 6) Power-up/Power-down sequence for the panel
> > 
> > But, we also have to see what all is feasible today from the DRM fwk
> > standpoint. There are some limitations about what is boot-time configurable
> > using bootparams and what is runtime configurable (across a modeset) using
> > debugfs.
> > 
> > 1) Today, everything part of struct mipi_dsi_device needs to be available at
> > boot time from what I can see as we need that while calling
> > mipi_dsi_attach(). So for that we went with boot-params.
> > 
> > 2) For the list of modes, we can move this to a debugfs like
> > "populate_modes" which the client using a sim panel can call before picking
> > a mode and triggering a commit.
> > 
> > But we need to have some default mode and configuration.
> 
> Uh, at the risk of sounding a bit like I'm just chasing the latest
> buzzwords, but this sounds like something that's screaming for ebpf.

I make a half-joke to Jani on IRC about it, but I was also being
half-serious. If the goal we want to have is to fully emulate any panel
variation, ebpf really looks like the best and most flexible way
forward.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH RFC 0/4] Support for Simulated Panels

[Jessica Zhang]

On 2/2/2024 2:15 AM, Maxime Ripard wrote:
> On Tue, Jan 30, 2024 at 09:53:13AM +0100, Daniel Vetter wrote:
>>>>>>> Wouldn't it be simpler if we had a vkms-like panel that we could either
>>>>>>> configure from DT or from debugfs that would just be registered the
>>>>>>> usual way and would be the only panel we register?
>>>>>>
>>>>>
>>>>> No, we need to have validate actual hardware pipeline with the simulated
>>>>> panel. With vkms, actual display pipeline will not be validated. With
>>>>> incorrect display pipeline misconfigurations arising from different panel
>>>>> combinations, this can easily be caught with any existing IGT CRC testing.
>>>>> In addition, all performance related bugs can also be easily caught by
>>>>> simulating high resolution displays.
>>>>
>>>> That's not what I meant. What I meant was that something like a
>>>> user-configurable, generic, panel driver would be a good idea. Just like
>>>> vkms (with the debugfs patches) is for a full blown KMS device.
>>>>
>>>
>>> Let me respond for both this question and the one below from you/Jani.
>>>
>>> Certainly having user-configurable information is a goal here. The end-goal
>>> is to make everything there in the existing panels such as below like I
>>> wrote:
>>>
>>> 1) Display resolution with timings (drm_display_mode)
>>> 2) Compression/non-compression
>>> 3) Command mode/Video mode
>>> 4) MIPI mode flags
>>> 5) DCS commands for panel enable/disable and other panel sequences
>>> 6) Power-up/Power-down sequence for the panel
>>>
>>> But, we also have to see what all is feasible today from the DRM fwk
>>> standpoint. There are some limitations about what is boot-time configurable
>>> using bootparams and what is runtime configurable (across a modeset) using
>>> debugfs.
>>>
>>> 1) Today, everything part of struct mipi_dsi_device needs to be available at
>>> boot time from what I can see as we need that while calling
>>> mipi_dsi_attach(). So for that we went with boot-params.
>>>
>>> 2) For the list of modes, we can move this to a debugfs like
>>> "populate_modes" which the client using a sim panel can call before picking
>>> a mode and triggering a commit.
>>>
>>> But we need to have some default mode and configuration.
>>
>> Uh, at the risk of sounding a bit like I'm just chasing the latest
>> buzzwords, but this sounds like something that's screaming for ebpf.
> 
> I make a half-joke to Jani on IRC about it, but I was also being
> half-serious. If the goal we want to have is to fully emulate any panel
> variation, ebpf really looks like the best and most flexible way
> forward.

Hi Maxime and Daniel,

For our current sim panel requirements, we can go with implementing the 
configfs first then add ebpf if requirements get more complex.

Thanks,

Jessica Zhang

> 
> Maxime

Re: [PATCH RFC 0/4] Support for Simulated Panels

[Daniel Vetter]

On Wed, Feb 28, 2024 at 01:49:34PM -0800, Jessica Zhang wrote:
> 
> 
> On 2/2/2024 2:15 AM, Maxime Ripard wrote:
> > On Tue, Jan 30, 2024 at 09:53:13AM +0100, Daniel Vetter wrote:
> > > > > > > > Wouldn't it be simpler if we had a vkms-like panel that we could either
> > > > > > > > configure from DT or from debugfs that would just be registered the
> > > > > > > > usual way and would be the only panel we register?
> > > > > > > 
> > > > > > 
> > > > > > No, we need to have validate actual hardware pipeline with the simulated
> > > > > > panel. With vkms, actual display pipeline will not be validated. With
> > > > > > incorrect display pipeline misconfigurations arising from different panel
> > > > > > combinations, this can easily be caught with any existing IGT CRC testing.
> > > > > > In addition, all performance related bugs can also be easily caught by
> > > > > > simulating high resolution displays.
> > > > > 
> > > > > That's not what I meant. What I meant was that something like a
> > > > > user-configurable, generic, panel driver would be a good idea. Just like
> > > > > vkms (with the debugfs patches) is for a full blown KMS device.
> > > > > 
> > > > 
> > > > Let me respond for both this question and the one below from you/Jani.
> > > > 
> > > > Certainly having user-configurable information is a goal here. The end-goal
> > > > is to make everything there in the existing panels such as below like I
> > > > wrote:
> > > > 
> > > > 1) Display resolution with timings (drm_display_mode)
> > > > 2) Compression/non-compression
> > > > 3) Command mode/Video mode
> > > > 4) MIPI mode flags
> > > > 5) DCS commands for panel enable/disable and other panel sequences
> > > > 6) Power-up/Power-down sequence for the panel
> > > > 
> > > > But, we also have to see what all is feasible today from the DRM fwk
> > > > standpoint. There are some limitations about what is boot-time configurable
> > > > using bootparams and what is runtime configurable (across a modeset) using
> > > > debugfs.
> > > > 
> > > > 1) Today, everything part of struct mipi_dsi_device needs to be available at
> > > > boot time from what I can see as we need that while calling
> > > > mipi_dsi_attach(). So for that we went with boot-params.
> > > > 
> > > > 2) For the list of modes, we can move this to a debugfs like
> > > > "populate_modes" which the client using a sim panel can call before picking
> > > > a mode and triggering a commit.
> > > > 
> > > > But we need to have some default mode and configuration.
> > > 
> > > Uh, at the risk of sounding a bit like I'm just chasing the latest
> > > buzzwords, but this sounds like something that's screaming for ebpf.
> > 
> > I make a half-joke to Jani on IRC about it, but I was also being
> > half-serious. If the goal we want to have is to fully emulate any panel
> > variation, ebpf really looks like the best and most flexible way
> > forward.
> 
> Hi Maxime and Daniel,
> 
> For our current sim panel requirements, we can go with implementing the
> configfs first then add ebpf if requirements get more complex.

Agreed, this is definitely the pragmatic approach to get this going.
-Sima
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch