[PATCH 0/4] sm7325-nothing-spacewar: Add and enable the panel

[PATCH 0/4] sm7325-nothing-spacewar: Add and enable the panel

[Danila Tikhonov]

This patch series adds support for the Visionox RM692E5 panel, which is
used on the Nothing Phone (1) and then adds it to the DTS.

But before adding to DTS we need to allow all bpc values ​​in DSC code,
because Visionox RM692E5 has a bpc value of 10. Also we need to make sure
that the DSC patch for 1.1.1 topology was applied.

To: Neil Armstrong <neil.armstrong@linaro.org> 
To: Jessica Zhang <quic_jesszhan@quicinc.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
To: Rob Clark <robdclark@gmail.com>
To: Abhinav Kumar <quic_abhinavk@quicinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Sean Paul <sean@poorly.run>
To: Marijn Suijten <marijn.suijten@somainline.org>
To: Jonathan Marek <jonathan@marek.ca>
To: Jun Nie <jun.nie@linaro.org>
To: Eugene Lepshy <fekz115@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: linux@mainlining.org
Cc: ~postmarketos/upstreaming@lists.sr.ht
Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>

Danila Tikhonov (1):
  dt-bindings: display: panel: Add Visionox RM692E5

Eugene Lepshy (3):
  drm/panel: Add Visionox RM692E5 panel driver
  drm/msm/dsi: Allow all bpc values
  arm64: dts: qcom: sm7325-nothing-spacewar: Enable panel and GPU

 .../display/panel/visionox,rm692e5.yaml       |  77 ++++
 .../boot/dts/qcom/sm7325-nothing-spacewar.dts |  53 ++-
 drivers/gpu/drm/msm/dsi/dsi_host.c            |   7 +-
 drivers/gpu/drm/panel/Kconfig                 |  10 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-visionox-rm692e5.c    | 433 ++++++++++++++++++
 6 files changed, 573 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/visionox,rm692e5.yaml
 create mode 100644 drivers/gpu/drm/panel/panel-visionox-rm692e5.c

-- 
2.48.1

[PATCH 1/4] dt-bindings: display: panel: Add Visionox RM692E5

[Danila Tikhonov]

The Visionox RM692E5 is a 6.55” AMOLED panel used in Nothing Phone (1)
(sm7325-nothing-spacewar).

Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
---
 .../display/panel/visionox,rm692e5.yaml       | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/visionox,rm692e5.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/visionox,rm692e5.yaml b/Documentation/devicetree/bindings/display/panel/visionox,rm692e5.yaml
new file mode 100644
index 000000000000..d4b4672815fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/visionox,rm692e5.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/visionox,rm692e5.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Visionox RM692E5 6.55" 2400x1080 120Hz MIPI-DSI Panel
+
+maintainers:
+  - Danila Tikhonov <danila@jiaxyga.com>
+
+description:
+  The Visionox RM692E5 is a generic DSI Panel IC used to control
+  AMOLED panels.
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - visionox,rm692e5
+      - items:
+          - enum:
+              - nothing,rm692e5-spacewar
+          - const: visionox,rm692e5
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: 3.3V source voltage rail
+
+  vddio-supply:
+    description: 1.8V I/O source voltage rail
+
+  reset-gpios: true
+  port: true
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+  - vdd-supply
+  - vddio-supply
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    dsi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        panel@0 {
+            compatible = "nothing,rm692e5-spacewar",
+                         "visionox,rm692e5";
+            reg = <0>;
+
+            reset-gpios = <&tlmm 44 GPIO_ACTIVE_LOW>;
+
+            vdd-supply = <&vdd_oled>;
+            vddio-supply = <&vdd_io_oled>;
+
+            port {
+                panel_in: endpoint {
+                    remote-endpoint = <&mdss_dsi0_out>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.48.1

[PATCH 2/4] drm/panel: Add Visionox RM692E5 panel driver

[Danila Tikhonov]

From: Eugene Lepshy <fekz115@gmail.com>

Add the driver for Visionox RM692E5 panel support found in Nothing
Phone (1).

Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
Co-developed-by: Danila Tikhonov <danila@jiaxyga.com>
Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
---
 drivers/gpu/drm/panel/Kconfig                 |  10 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-visionox-rm692e5.c    | 433 ++++++++++++++++++
 3 files changed, 444 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-visionox-rm692e5.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index d7469c565d1d..66b2827169fb 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -996,6 +996,16 @@ config DRM_PANEL_VISIONOX_RM69299
 	  Say Y here if you want to enable support for Visionox
 	  RM69299  DSI Video Mode panel.
 
+config DRM_PANEL_VISIONOX_RM692E5
+	tristate "Visionox RM692E5"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for Visionox RM692E5 amoled
+	  display panels, such as the one found in the Nothing Phone (1)
+	  smartphone.
+
 config DRM_PANEL_VISIONOX_VTDR6130
 	tristate "Visionox VTDR6130"
 	depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 7dcf72646cac..6177f2d4113e 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
 obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
 obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
 obj-$(CONFIG_DRM_PANEL_VISIONOX_RM69299) += panel-visionox-rm69299.o
+obj-$(CONFIG_DRM_PANEL_VISIONOX_RM692E5) += panel-visionox-rm692e5.o
 obj-$(CONFIG_DRM_PANEL_VISIONOX_VTDR6130) += panel-visionox-vtdr6130.o
 obj-$(CONFIG_DRM_PANEL_VISIONOX_R66451) += panel-visionox-r66451.o
 obj-$(CONFIG_DRM_PANEL_WIDECHIPS_WS2401) += panel-widechips-ws2401.o
diff --git a/drivers/gpu/drm/panel/panel-visionox-rm692e5.c b/drivers/gpu/drm/panel/panel-visionox-rm692e5.c
new file mode 100644
index 000000000000..abaa035f3a92
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-visionox-rm692e5.c
@@ -0,0 +1,433 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree:
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2025, Eugene Lepshy <fekz115@gmail.com>
+ * Copyright (c) 2025, Danila Tikhonov <danila@jiaxyga.com>
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+
+#include <video/mipi_display.h>
+
+#include <drm/display/drm_dsc.h>
+#include <drm/display/drm_dsc_helper.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_probe_helper.h>
+
+struct visionox_rm692e5 {
+	struct drm_panel panel;
+	struct mipi_dsi_device *dsi;
+	struct drm_dsc_config dsc;
+	struct gpio_desc *reset_gpio;
+	struct regulator_bulk_data *supplies;
+};
+
+static const struct regulator_bulk_data visionox_rm692e5_supplies[] = {
+	{ .supply = "vddio" },	/* 1p8 */
+	{ .supply = "vdd" },	/* 3p3 */
+};
+
+static inline
+struct visionox_rm692e5 *to_visionox_rm692e5(struct drm_panel *panel)
+{
+	return container_of(panel, struct visionox_rm692e5, panel);
+}
+
+static void visionox_rm692e5_reset(struct visionox_rm692e5 *ctx)
+{
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	usleep_range(10000, 11000);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	usleep_range(1000, 2000);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	msleep(32);
+}
+
+static int visionox_rm692e5_on(struct visionox_rm692e5 *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x40);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xbd, 0x07);
+	mipi_dsi_usleep_range(&dsi_ctx, 17000, 18000);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xd2);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x50, 0x11);
+	mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, 0x00ab);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x52, 0x30);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x09);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x54, 0x60);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x04);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x56, 0x38);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x58, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x59, 0x14);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5a, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5b, 0x1c);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5c, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5d, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_SET_CABC_MIN_BRIGHTNESS, 0x20);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5f, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x60, 0xe8);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x61, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x62, 0x07);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x63, 0x0c);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x64, 0x05);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x65, 0x0e);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x66, 0x05);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x67, 0x16);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x68, 0x18);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x69, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6a, 0x10);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6b, 0xf0);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6c, 0x07);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6d, 0x10);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6e, 0x20);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x70, 0x06);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x71, 0x0f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x72, 0x0f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x73, 0x33);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x74, 0x0e);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x75, 0x1c);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x76, 0x2a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x77, 0x38);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x78, 0x46);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x79, 0x54);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7a, 0x62);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7b, 0x69);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7c, 0x70);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7d, 0x77);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7e, 0x79);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7f, 0x7b);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x80, 0x7d);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x81, 0x7e);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x82, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x83, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x84, 0x22);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x85, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x86, 0x2a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x87, 0x40);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x88, 0x2a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x89, 0xbe);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8a, 0x3a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8b, 0xfc);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8c, 0x3a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8d, 0xfa);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8e, 0x3a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8f, 0xf8);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x90, 0x3b);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x91, 0x38);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x92, 0x3b);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x93, 0x78);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x94, 0x3b);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x95, 0xb6);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x96, 0x4b);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x97, 0xf6);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x98, 0x4c);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x99, 0x34);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9a, 0x4c);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9b, 0x74);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9c, 0x5c);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9d, 0x74);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9e, 0x8c);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9f, 0xf4);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_READ_PPS_START, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa3, 0x1c);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa4, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa5, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa6, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa7, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_READ_PPS_CONTINUE, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xaa, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa0, 0x80);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xa1);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x6b);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xce, 0xbb);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xd1);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb4, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x38);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x17, 0x0f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x18, 0x0f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfa, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc2, 0x08);
+	mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+	mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, 0x000d);
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 50);
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
+
+	return dsi_ctx.accum_err;
+}
+
+static int visionox_rm692e5_disable(struct drm_panel *panel)
+{
+	struct visionox_rm692e5 *ctx = to_visionox_rm692e5(panel);
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
+
+	return dsi_ctx.accum_err;
+}
+
+static int visionox_rm692e5_prepare(struct drm_panel *panel)
+{
+	struct visionox_rm692e5 *ctx = to_visionox_rm692e5(panel);
+	struct drm_dsc_picture_parameter_set pps;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
+
+	dsi_ctx.accum_err = regulator_bulk_enable(ARRAY_SIZE
+				(visionox_rm692e5_supplies), ctx->supplies);
+	if (dsi_ctx.accum_err)
+		return dsi_ctx.accum_err;
+
+	visionox_rm692e5_reset(ctx);
+	visionox_rm692e5_on(ctx);
+	drm_dsc_pps_payload_pack(&pps, &ctx->dsc);
+	mipi_dsi_picture_parameter_set_multi(&dsi_ctx, &pps);
+	mipi_dsi_compression_mode_ext_multi(&dsi_ctx, true, MIPI_DSI_COMPRESSION_DSC, 0);
+
+	mipi_dsi_msleep(&dsi_ctx, 28);
+
+	if (dsi_ctx.accum_err) {
+		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+		regulator_bulk_disable(ARRAY_SIZE(visionox_rm692e5_supplies),
+				       ctx->supplies);
+	}
+
+	return dsi_ctx.accum_err;
+}
+
+static int visionox_rm692e5_unprepare(struct drm_panel *panel)
+{
+	struct visionox_rm692e5 *ctx = to_visionox_rm692e5(panel);
+
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	regulator_bulk_disable(ARRAY_SIZE(visionox_rm692e5_supplies),
+			       ctx->supplies);
+
+	return 0;
+}
+
+static const struct drm_display_mode visionox_rm692e5_modes[] = {
+	/* Let's initialize the highest frequency first */
+	{ /* 120Hz mode */
+		.clock = (1080 + 26 + 39 + 36) * (2400 + 16 + 21 + 16) * 120 / 1000,
+		.hdisplay = 1080,
+		.hsync_start = 1080 + 26,
+		.hsync_end = 1080 + 26 + 39,
+		.htotal = 1080 + 26 + 39 + 36,
+		.vdisplay = 2400,
+		.vsync_start = 2400 + 16,
+		.vsync_end = 2400 + 16 + 21,
+		.vtotal = 2400 + 16 + 21 + 16,
+		.width_mm = 68,
+		.height_mm = 152,
+		.type = DRM_MODE_TYPE_DRIVER,
+	},
+	{ /* 90Hz mode */
+		.clock = (1080 + 26 + 39 + 36) * (2400 + 16 + 21 + 16) * 90 / 1000,
+		.hdisplay = 1080,
+		.hsync_start = 1080 + 26,
+		.hsync_end = 1080 + 26 + 39,
+		.htotal = 1080 + 26 + 39 + 36,
+		.vdisplay = 2400,
+		.vsync_start = 2400 + 16,
+		.vsync_end = 2400 + 16 + 21,
+		.vtotal = 2400 + 16 + 21 + 16,
+		.width_mm = 68,
+		.height_mm = 152,
+		.type = DRM_MODE_TYPE_DRIVER,
+	},
+	{ /* 60Hz mode */
+		.clock = (1080 + 26 + 39 + 36) * (2400 + 16 + 21 + 16) * 60 / 1000,
+		.hdisplay = 1080,
+		.hsync_start = 1080 + 26,
+		.hsync_end = 1080 + 26 + 39,
+		.htotal = 1080 + 26 + 39 + 36,
+		.vdisplay = 2400,
+		.vsync_start = 2400 + 16,
+		.vsync_end = 2400 + 16 + 21,
+		.vtotal = 2400 + 16 + 21 + 16,
+		.width_mm = 68,
+		.height_mm = 152,
+		.type = DRM_MODE_TYPE_DRIVER,
+	},
+};
+
+static int visionox_rm692e5_get_modes(struct drm_panel *panel,
+						   struct drm_connector *connector)
+{
+	int count = 0;
+
+	for (int i = 0; i < ARRAY_SIZE(visionox_rm692e5_modes); i++) {
+		count += drm_connector_helper_get_modes_fixed(connector,
+						    &visionox_rm692e5_modes[i]);
+	}
+
+	return count;
+}
+
+static const struct drm_panel_funcs visionox_rm692e5_panel_funcs = {
+	.prepare = visionox_rm692e5_prepare,
+	.unprepare = visionox_rm692e5_unprepare,
+	.disable = visionox_rm692e5_disable,
+	.get_modes = visionox_rm692e5_get_modes,
+};
+
+static int visionox_rm692e5_bl_update_status(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	u16 brightness = backlight_get_brightness(bl);
+	int ret;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	return 0;
+}
+
+static int visionox_rm692e5_bl_get_brightness(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	u16 brightness;
+	int ret;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_get_display_brightness_large(dsi, &brightness);
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	return brightness;
+}
+
+static const struct backlight_ops visionox_rm692e5_bl_ops = {
+	.update_status = visionox_rm692e5_bl_update_status,
+	.get_brightness = visionox_rm692e5_bl_get_brightness,
+};
+
+static struct backlight_device *
+visionox_rm692e5_create_backlight(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	const struct backlight_properties props = {
+		.type = BACKLIGHT_RAW,
+		.brightness = 2047,
+		.max_brightness = 4095,
+	};
+
+	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+					      &visionox_rm692e5_bl_ops, &props);
+}
+
+static int visionox_rm692e5_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct visionox_rm692e5 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ret = devm_regulator_bulk_get_const(&dsi->dev,
+					    ARRAY_SIZE(visionox_rm692e5_supplies),
+					    visionox_rm692e5_supplies,
+					    &ctx->supplies);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get regulators\n");
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ctx->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
+				     "Failed to get reset-gpios\n");
+
+	ctx->dsi = dsi;
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS;
+
+	drm_panel_init(&ctx->panel, dev, &visionox_rm692e5_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+	ctx->panel.prepare_prev_first = true;
+
+	ctx->panel.backlight = visionox_rm692e5_create_backlight(dsi);
+	if (IS_ERR(ctx->panel.backlight))
+		return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
+				     "Failed to create backlight\n");
+
+	drm_panel_add(&ctx->panel);
+
+	dsi->dsc = &ctx->dsc;
+	ctx->dsc.dsc_version_major = 1;
+	ctx->dsc.dsc_version_minor = 1;
+	ctx->dsc.slice_height = 20;
+	ctx->dsc.slice_width = 540;
+	ctx->dsc.slice_count = 1080 / ctx->dsc.slice_width;
+	ctx->dsc.bits_per_component = 10;
+	ctx->dsc.bits_per_pixel = 8 << 4;
+	ctx->dsc.block_pred_enable = true;
+
+	ret = devm_mipi_dsi_attach(dev, dsi);
+	if (ret < 0) {
+		drm_panel_remove(&ctx->panel);
+		return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
+	}
+
+	return 0;
+}
+
+static void visionox_rm692e5_remove(struct mipi_dsi_device *dsi)
+{
+	struct visionox_rm692e5 *ctx = mipi_dsi_get_drvdata(dsi);
+
+	drm_panel_remove(&ctx->panel);
+}
+
+static const struct of_device_id visionox_rm692e5_of_match[] = {
+	{ .compatible = "visionox,rm692e5" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, visionox_rm692e5_of_match);
+
+static struct mipi_dsi_driver visionox_rm692e5_driver = {
+	.probe = visionox_rm692e5_probe,
+	.remove = visionox_rm692e5_remove,
+	.driver = {
+		.name = "panel-visionox-rm692e5",
+		.of_match_table = visionox_rm692e5_of_match,
+	},
+};
+module_mipi_dsi_driver(visionox_rm692e5_driver);
+
+MODULE_AUTHOR("Eugene Lepshy <fekz115@gmail.com>");
+MODULE_AUTHOR("Danila Tikhonov <danila@jiaxyga.com>");
+MODULE_DESCRIPTION("DRM driver for Visionox RM692E5 cmd mode dsi panel");
+MODULE_LICENSE("GPL");
-- 
2.48.1

[PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Danila Tikhonov]

From: Eugene Lepshy <fekz115@gmail.com>

DRM DSC helper has parameters for various bpc values ​​other than 8:
(8/10/12/14/16).

Remove this guard.

Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
---
 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 007311c21fda..d182af7bbb81 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1767,11 +1767,6 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
 		return -EINVAL;
 	}
 
-	if (dsc->bits_per_component != 8) {
-		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
-		return -EOPNOTSUPP;
-	}
-
 	dsc->simple_422 = 0;
 	dsc->convert_rgb = 1;
 	dsc->vbr_enable = 0;
@@ -1779,7 +1774,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
 	drm_dsc_set_const_params(dsc);
 	drm_dsc_set_rc_buf_thresh(dsc);
 
-	/* handle only bpp = bpc = 8, pre-SCR panels */
+	/* handle only pre-SCR panels */
 	ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
 	if (ret) {
 		DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters\n");
-- 
2.48.1

[PATCH 4/4] arm64: dts: qcom: sm7325-nothing-spacewar: Enable panel and GPU

[Danila Tikhonov]

From: Eugene Lepshy <fekz115@gmail.com>

Enable the Adreno GPU and configure the Visionox RM692E5 panel.

Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
Co-developed-by: Danila Tikhonov <danila@jiaxyga.com>
Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
---
Note:
Depends on https://lore.kernel.org/linux-arm-msm/20250122-dpu-111-topology-v2-1-505e95964af9@somainline.org/
---
 .../boot/dts/qcom/sm7325-nothing-spacewar.dts | 53 ++++++++++++++++++-
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
index a5cda478bd78..cda317b49d5c 100644
--- a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
+++ b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
@@ -52,6 +52,8 @@ framebuffer0: framebuffer@e1000000 {
 			stride = <(1080 * 4)>;
 			format = "a8r8g8b8";
 
+			display = <&panel0>;
+
 			clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
 				 <&dispcc DISP_CC_MDSS_MDP_CLK>,
 				 <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
@@ -757,6 +759,10 @@ &gpi_dma1 {
 	status = "okay";
 };
 
+&gpu {
+	status = "okay";
+};
+
 &gpu_zap_shader {
 	firmware-name = "qcom/sm7325/nothing/spacewar/a660_zap.mbn";
 };
@@ -823,15 +829,44 @@ &ipa {
 	status = "okay";
 };
 
-/* MDSS remains disabled until the panel driver is present. */
+&mdss {
+	status = "okay";
+};
+
 &mdss_dsi {
 	vdda-supply = <&vdd_a_dsi_0_1p2>;
+	status = "okay";
 
-	/* Visionox RM692E5 panel */
+	panel0: panel@0 {
+		compatible = "nothing,rm692e5-spacewar",
+			     "visionox,rm692e5";
+		reg = <0>;
+
+		reset-gpios = <&tlmm 44 GPIO_ACTIVE_LOW>;
+
+		vdd-supply = <&vdd_oled>;
+		vddio-supply = <&vdd_io_oled>;
+
+		pinctrl-0 = <&lcd_reset_n>,
+			    <&mdp_vsync_p>;
+		pinctrl-names = "default";
+
+		port {
+			panel_in: endpoint {
+				remote-endpoint = <&mdss_dsi0_out>;
+			};
+		};
+	};
+};
+
+&mdss_dsi0_out {
+	data-lanes = <0 1 2 3>;
+	remote-endpoint = <&panel_in>;
 };
 
 &mdss_dsi_phy {
 	vdds-supply = <&vdd_a_dsi_0_0p9>;
+	status = "okay";
 };
 
 &pm7325_gpios {
@@ -1147,6 +1182,20 @@ nfc_int_req: nfc-int-req-state {
 		bias-pull-down;
 	};
 
+	lcd_reset_n: lcd-reset-n-state {
+		pins = "gpio44";
+		function = "gpio";
+		drive-strength = <8>;
+		bias-disable;
+	};
+
+	mdp_vsync_p: mdp-vsync-p-state {
+		pins = "gpio80";
+		function = "mdp_vsync";
+		drive-strength = <2>;
+		bias-pull-down;
+	};
+
 	hst_bt_en: hst-bt-en-state {
 		pins = "gpio85";
 		function = "gpio";
-- 
2.48.1

Re: [PATCH 1/4] dt-bindings: display: panel: Add Visionox RM692E5

[Rob Herring (Arm)]

On Mon, 03 Feb 2025 21:14:24 +0300, Danila Tikhonov wrote:
> The Visionox RM692E5 is a 6.55” AMOLED panel used in Nothing Phone (1)
> (sm7325-nothing-spacewar).
> 
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
>  .../display/panel/visionox,rm692e5.yaml       | 77 +++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/visionox,rm692e5.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Dmitry Baryshkov]

On Mon, Feb 03, 2025 at 09:14:26PM +0300, Danila Tikhonov wrote:
> From: Eugene Lepshy <fekz115@gmail.com>
> 
> DRM DSC helper has parameters for various bpc values ​​other than 8:
> (8/10/12/14/16).
> 
> Remove this guard.
> 
> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH 2/4] drm/panel: Add Visionox RM692E5 panel driver

[neil.armstrong]

Hi,

On 03/02/2025 19:14, Danila Tikhonov wrote:
> From: Eugene Lepshy <fekz115@gmail.com>
> 
> Add the driver for Visionox RM692E5 panel support found in Nothing
> Phone (1).
> 
> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> Co-developed-by: Danila Tikhonov <danila@jiaxyga.com>
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
>   drivers/gpu/drm/panel/Kconfig                 |  10 +
>   drivers/gpu/drm/panel/Makefile                |   1 +
>   .../gpu/drm/panel/panel-visionox-rm692e5.c    | 433 ++++++++++++++++++
>   3 files changed, 444 insertions(+)
>   create mode 100644 drivers/gpu/drm/panel/panel-visionox-rm692e5.c
> 
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index d7469c565d1d..66b2827169fb 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -996,6 +996,16 @@ config DRM_PANEL_VISIONOX_RM69299
>   	  Say Y here if you want to enable support for Visionox
>   	  RM69299  DSI Video Mode panel.
>   
> +config DRM_PANEL_VISIONOX_RM692E5
> +	tristate "Visionox RM692E5"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y here if you want to enable support for Visionox RM692E5 amoled
> +	  display panels, such as the one found in the Nothing Phone (1)
> +	  smartphone.
> +
>   config DRM_PANEL_VISIONOX_VTDR6130
>   	tristate "Visionox VTDR6130"
>   	depends on OF
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 7dcf72646cac..6177f2d4113e 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -100,6 +100,7 @@ obj-$(CONFIG_DRM_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
>   obj-$(CONFIG_DRM_PANEL_TPO_TPG110) += panel-tpo-tpg110.o
>   obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
>   obj-$(CONFIG_DRM_PANEL_VISIONOX_RM69299) += panel-visionox-rm69299.o
> +obj-$(CONFIG_DRM_PANEL_VISIONOX_RM692E5) += panel-visionox-rm692e5.o
>   obj-$(CONFIG_DRM_PANEL_VISIONOX_VTDR6130) += panel-visionox-vtdr6130.o
>   obj-$(CONFIG_DRM_PANEL_VISIONOX_R66451) += panel-visionox-r66451.o
>   obj-$(CONFIG_DRM_PANEL_WIDECHIPS_WS2401) += panel-widechips-ws2401.o
> diff --git a/drivers/gpu/drm/panel/panel-visionox-rm692e5.c b/drivers/gpu/drm/panel/panel-visionox-rm692e5.c
> new file mode 100644
> index 000000000000..abaa035f3a92
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-visionox-rm692e5.c
> @@ -0,0 +1,433 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generated with linux-mdss-dsi-panel-driver-generator from vendor device tree:
> + * Copyright (c) 2013, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2025, Eugene Lepshy <fekz115@gmail.com>
> + * Copyright (c) 2025, Danila Tikhonov <danila@jiaxyga.com>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <video/mipi_display.h>
> +
> +#include <drm/display/drm_dsc.h>
> +#include <drm/display/drm_dsc_helper.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_probe_helper.h>
> +
> +struct visionox_rm692e5 {
> +	struct drm_panel panel;
> +	struct mipi_dsi_device *dsi;
> +	struct drm_dsc_config dsc;
> +	struct gpio_desc *reset_gpio;
> +	struct regulator_bulk_data *supplies;
> +};
> +
> +static const struct regulator_bulk_data visionox_rm692e5_supplies[] = {
> +	{ .supply = "vddio" },	/* 1p8 */
> +	{ .supply = "vdd" },	/* 3p3 */
> +};
> +
> +static inline
> +struct visionox_rm692e5 *to_visionox_rm692e5(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct visionox_rm692e5, panel);
> +}
> +
> +static void visionox_rm692e5_reset(struct visionox_rm692e5 *ctx)
> +{
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	usleep_range(10000, 11000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
> +	msleep(32);
> +}
> +
> +static int visionox_rm692e5_on(struct visionox_rm692e5 *ctx)
> +{
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xbd, 0x07);
> +	mipi_dsi_usleep_range(&dsi_ctx, 17000, 18000);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xd2);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x50, 0x11);
> +	mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, 0x00ab);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x52, 0x30);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, 0x09);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x54, 0x60);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x04);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x56, 0x38);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x58, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x59, 0x14);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5a, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5b, 0x1c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5c, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5d, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_SET_CABC_MIN_BRIGHTNESS, 0x20);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x5f, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x60, 0xe8);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x61, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x62, 0x07);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x63, 0x0c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x64, 0x05);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x65, 0x0e);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x66, 0x05);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x67, 0x16);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x68, 0x18);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x69, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6a, 0x10);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6b, 0xf0);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6c, 0x07);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6d, 0x10);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6e, 0x20);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x6f, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x70, 0x06);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x71, 0x0f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x72, 0x0f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x73, 0x33);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x74, 0x0e);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x75, 0x1c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x76, 0x2a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x77, 0x38);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x78, 0x46);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x79, 0x54);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7a, 0x62);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7b, 0x69);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7c, 0x70);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7d, 0x77);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7e, 0x79);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x7f, 0x7b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x80, 0x7d);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x81, 0x7e);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x82, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x83, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x84, 0x22);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x85, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x86, 0x2a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x87, 0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x88, 0x2a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x89, 0xbe);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8a, 0x3a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8b, 0xfc);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8c, 0x3a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8d, 0xfa);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8e, 0x3a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x8f, 0xf8);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x90, 0x3b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x91, 0x38);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x92, 0x3b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x93, 0x78);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x94, 0x3b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x95, 0xb6);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x96, 0x4b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x97, 0xf6);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x98, 0x4c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x99, 0x34);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9a, 0x4c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9b, 0x74);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9c, 0x5c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9d, 0x74);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9e, 0x8c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x9f, 0xf4);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_READ_PPS_START, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa3, 0x1c);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa4, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa5, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa6, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa7, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, MIPI_DCS_READ_PPS_CONTINUE, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xaa, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xa0, 0x80);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xa1);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xcd, 0x6b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xce, 0xbb);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0xd1);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xb4, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x38);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x17, 0x0f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0x18, 0x0f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfe, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xfa, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, 0xc2, 0x08);
> +	mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> +	mipi_dsi_dcs_set_display_brightness_multi(&dsi_ctx, 0x000d);
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 50);
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
> +
> +	return dsi_ctx.accum_err;
> +}
> +
> +static int visionox_rm692e5_disable(struct drm_panel *panel)
> +{
> +	struct visionox_rm692e5 *ctx = to_visionox_rm692e5(panel);
> +	struct mipi_dsi_device *dsi = ctx->dsi;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> +
> +	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_usleep_range(&dsi_ctx, 1000, 2000);
> +
> +	return dsi_ctx.accum_err;
> +}
> +
> +static int visionox_rm692e5_prepare(struct drm_panel *panel)
> +{
> +	struct visionox_rm692e5 *ctx = to_visionox_rm692e5(panel);
> +	struct drm_dsc_picture_parameter_set pps;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	dsi_ctx.accum_err = regulator_bulk_enable(ARRAY_SIZE
> +				(visionox_rm692e5_supplies), ctx->supplies);
> +	if (dsi_ctx.accum_err)
> +		return dsi_ctx.accum_err;

This is quite unpretty, just add a additional ret, the compiler will
optimize things much smarter, keep code readable.

> +
> +	visionox_rm692e5_reset(ctx);
> +	visionox_rm692e5_on(ctx);

You don't check the return here, just pass the current dsi_ctx

> +	drm_dsc_pps_payload_pack(&pps, &ctx->dsc);
> +	mipi_dsi_picture_parameter_set_multi(&dsi_ctx, &pps);
> +	mipi_dsi_compression_mode_ext_multi(&dsi_ctx, true, MIPI_DSI_COMPRESSION_DSC, 0);
> +
> +	mipi_dsi_msleep(&dsi_ctx, 28);
> +
> +	if (dsi_ctx.accum_err) {
> +		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +		regulator_bulk_disable(ARRAY_SIZE(visionox_rm692e5_supplies),
> +				       ctx->supplies);
> +	}
> +
> +	return dsi_ctx.accum_err;
> +}
> +
> +static int visionox_rm692e5_unprepare(struct drm_panel *panel)
> +{
> +	struct visionox_rm692e5 *ctx = to_visionox_rm692e5(panel);
> +
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +	regulator_bulk_disable(ARRAY_SIZE(visionox_rm692e5_supplies),
> +			       ctx->supplies);
> +
> +	return 0;
> +}
> +
> +static const struct drm_display_mode visionox_rm692e5_modes[] = {
> +	/* Let's initialize the highest frequency first */
> +	{ /* 120Hz mode */
> +		.clock = (1080 + 26 + 39 + 36) * (2400 + 16 + 21 + 16) * 120 / 1000,
> +		.hdisplay = 1080,
> +		.hsync_start = 1080 + 26,
> +		.hsync_end = 1080 + 26 + 39,
> +		.htotal = 1080 + 26 + 39 + 36,
> +		.vdisplay = 2400,
> +		.vsync_start = 2400 + 16,
> +		.vsync_end = 2400 + 16 + 21,
> +		.vtotal = 2400 + 16 + 21 + 16,
> +		.width_mm = 68,
> +		.height_mm = 152,
> +		.type = DRM_MODE_TYPE_DRIVER,
> +	},
> +	{ /* 90Hz mode */
> +		.clock = (1080 + 26 + 39 + 36) * (2400 + 16 + 21 + 16) * 90 / 1000,
> +		.hdisplay = 1080,
> +		.hsync_start = 1080 + 26,
> +		.hsync_end = 1080 + 26 + 39,
> +		.htotal = 1080 + 26 + 39 + 36,
> +		.vdisplay = 2400,
> +		.vsync_start = 2400 + 16,
> +		.vsync_end = 2400 + 16 + 21,
> +		.vtotal = 2400 + 16 + 21 + 16,
> +		.width_mm = 68,
> +		.height_mm = 152,
> +		.type = DRM_MODE_TYPE_DRIVER,
> +	},
> +	{ /* 60Hz mode */
> +		.clock = (1080 + 26 + 39 + 36) * (2400 + 16 + 21 + 16) * 60 / 1000,
> +		.hdisplay = 1080,
> +		.hsync_start = 1080 + 26,
> +		.hsync_end = 1080 + 26 + 39,
> +		.htotal = 1080 + 26 + 39 + 36,
> +		.vdisplay = 2400,
> +		.vsync_start = 2400 + 16,
> +		.vsync_end = 2400 + 16 + 21,
> +		.vtotal = 2400 + 16 + 21 + 16,
> +		.width_mm = 68,
> +		.height_mm = 152,
> +		.type = DRM_MODE_TYPE_DRIVER,
> +	},
> +};
> +
> +static int visionox_rm692e5_get_modes(struct drm_panel *panel,
> +						   struct drm_connector *connector)
> +{
> +	int count = 0;
> +
> +	for (int i = 0; i < ARRAY_SIZE(visionox_rm692e5_modes); i++) {
> +		count += drm_connector_helper_get_modes_fixed(connector,
> +						    &visionox_rm692e5_modes[i]);
> +	}

Nit: you can drop those {}

> +
> +	return count;
> +}
> +
> +static const struct drm_panel_funcs visionox_rm692e5_panel_funcs = {
> +	.prepare = visionox_rm692e5_prepare,
> +	.unprepare = visionox_rm692e5_unprepare,
> +	.disable = visionox_rm692e5_disable,
> +	.get_modes = visionox_rm692e5_get_modes,
> +};
> +
> +static int visionox_rm692e5_bl_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	u16 brightness = backlight_get_brightness(bl);
> +	int ret;
> +
> +	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	return 0;
> +}
> +
> +static int visionox_rm692e5_bl_get_brightness(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	u16 brightness;
> +	int ret;
> +
> +	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +
> +	ret = mipi_dsi_dcs_get_display_brightness_large(dsi, &brightness);
> +	if (ret < 0)
> +		return ret;
> +
> +	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +
> +	return brightness;
> +}
> +
> +static const struct backlight_ops visionox_rm692e5_bl_ops = {
> +	.update_status = visionox_rm692e5_bl_update_status,
> +	.get_brightness = visionox_rm692e5_bl_get_brightness,
> +};
> +
> +static struct backlight_device *
> +visionox_rm692e5_create_backlight(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	const struct backlight_properties props = {
> +		.type = BACKLIGHT_RAW,
> +		.brightness = 2047,
> +		.max_brightness = 4095,
> +	};
> +
> +	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
> +					      &visionox_rm692e5_bl_ops, &props);
> +}
> +
> +static int visionox_rm692e5_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct visionox_rm692e5 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ret = devm_regulator_bulk_get_const(&dsi->dev,
> +					    ARRAY_SIZE(visionox_rm692e5_supplies),
> +					    visionox_rm692e5_supplies,
> +					    &ctx->supplies);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to get regulators\n");
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(ctx->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
> +				     "Failed to get reset-gpios\n");
> +
> +	ctx->dsi = dsi;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS;
> +
> +	drm_panel_init(&ctx->panel, dev, &visionox_rm692e5_panel_funcs,
> +		       DRM_MODE_CONNECTOR_DSI);
> +	ctx->panel.prepare_prev_first = true;
> +
> +	ctx->panel.backlight = visionox_rm692e5_create_backlight(dsi);
> +	if (IS_ERR(ctx->panel.backlight))
> +		return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
> +				     "Failed to create backlight\n");
> +
> +	drm_panel_add(&ctx->panel);
> +
> +	dsi->dsc = &ctx->dsc;
> +	ctx->dsc.dsc_version_major = 1;
> +	ctx->dsc.dsc_version_minor = 1;
> +	ctx->dsc.slice_height = 20;
> +	ctx->dsc.slice_width = 540;
> +	ctx->dsc.slice_count = 1080 / ctx->dsc.slice_width;
> +	ctx->dsc.bits_per_component = 10;
> +	ctx->dsc.bits_per_pixel = 8 << 4;
> +	ctx->dsc.block_pred_enable = true;
> +
> +	ret = devm_mipi_dsi_attach(dev, dsi);
> +	if (ret < 0) {
> +		drm_panel_remove(&ctx->panel);
> +		return dev_err_probe(dev, ret, "Failed to attach to DSI host\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static void visionox_rm692e5_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct visionox_rm692e5 *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	drm_panel_remove(&ctx->panel);
> +}
> +
> +static const struct of_device_id visionox_rm692e5_of_match[] = {
> +	{ .compatible = "visionox,rm692e5" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, visionox_rm692e5_of_match);
> +
> +static struct mipi_dsi_driver visionox_rm692e5_driver = {
> +	.probe = visionox_rm692e5_probe,
> +	.remove = visionox_rm692e5_remove,
> +	.driver = {
> +		.name = "panel-visionox-rm692e5",
> +		.of_match_table = visionox_rm692e5_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(visionox_rm692e5_driver);
> +
> +MODULE_AUTHOR("Eugene Lepshy <fekz115@gmail.com>");
> +MODULE_AUTHOR("Danila Tikhonov <danila@jiaxyga.com>");
> +MODULE_DESCRIPTION("DRM driver for Visionox RM692E5 cmd mode dsi panel");
> +MODULE_LICENSE("GPL");

Thanks,
Neil

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Marijn Suijten]

On 2025-02-03 21:14:26, Danila Tikhonov wrote:
> From: Eugene Lepshy <fekz115@gmail.com>
> 
> DRM DSC helper has parameters for various bpc values ​​other than 8:

Weird zero-width \u200b spaces here between "values" and "other", please delete
those.

> (8/10/12/14/16).
> 
> Remove this guard.
> 
> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>

Should this patch elaborate that those "DRM DSC helper" don't have any
additional guarding for the values you mention either, i.e. passing 9 or 11 or
>16 don't seem to be checked anywhere else either?

And your title might have space to spell out "Bits Per Component" entirely.

> ---
>  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 007311c21fda..d182af7bbb81 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1767,11 +1767,6 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
>  		return -EINVAL;
>  	}
>  
> -	if (dsc->bits_per_component != 8) {
> -		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
> -		return -EOPNOTSUPP;
> -	}
> -
>  	dsc->simple_422 = 0;
>  	dsc->convert_rgb = 1;
>  	dsc->vbr_enable = 0;

This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in
dpu_hw_dsc_config(), which has:

	data |= (dsc->line_buf_depth << 3);
	data |= (dsc->simple_422 << 2);
	data |= (dsc->convert_rgb << 1);
	data |= dsc->bits_per_component;

The original value of `8` would overlap with the lowest bit of line_buf_depth
(4th bit in `data`).  Now, the 2nd bit which will take the value from
convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in
your new bpc value of 10.

Can you double-check that this code in DPU1 is actually valid?  I assume you
have tested this panel at least and it is working (worthy mention in the cover
letter?), this just seems like yet another mistake in the original bindings
(though the register always had a matching value with downstream on 8 BPC panels
for me).

> @@ -1779,7 +1774,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
>  	drm_dsc_set_const_params(dsc);
>  	drm_dsc_set_rc_buf_thresh(dsc);
>  
> -	/* handle only bpp = bpc = 8, pre-SCR panels */
> +	/* handle only pre-SCR panels */
>  	ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);

Good catch - this comment sounds like it's documenting a limitation of
this helper function, but the function does not have such limitations...
rc_parameters_pre_scr has values for all these combinations.

- Marijn

>  	if (ret) {
>  		DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters\n");
> -- 
> 2.48.1
> 

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Dmitry Baryshkov]

On Sat, Feb 08, 2025 at 11:09:56PM +0100, Marijn Suijten wrote:
> On 2025-02-03 21:14:26, Danila Tikhonov wrote:
> > From: Eugene Lepshy <fekz115@gmail.com>
> > 
> > DRM DSC helper has parameters for various bpc values ​​other than 8:
> 
> Weird zero-width \u200b spaces here between "values" and "other", please delete
> those.
> 
> > (8/10/12/14/16).
> > 
> > Remove this guard.
> > 
> > Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> > Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> 
> Should this patch elaborate that those "DRM DSC helper" don't have any
> additional guarding for the values you mention either, i.e. passing 9 or 11 or
> >16 don't seem to be checked anywhere else either?
> 
> And your title might have space to spell out "Bits Per Component" entirely.
> 
> > ---
> >  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 007311c21fda..d182af7bbb81 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -1767,11 +1767,6 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (dsc->bits_per_component != 8) {
> > -		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
> > -		return -EOPNOTSUPP;
> > -	}
> > -
> >  	dsc->simple_422 = 0;
> >  	dsc->convert_rgb = 1;
> >  	dsc->vbr_enable = 0;
> 
> This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in
> dpu_hw_dsc_config(), which has:
> 
> 	data |= (dsc->line_buf_depth << 3);
> 	data |= (dsc->simple_422 << 2);
> 	data |= (dsc->convert_rgb << 1);
> 	data |= dsc->bits_per_component;
> 
> The original value of `8` would overlap with the lowest bit of line_buf_depth
> (4th bit in `data`).  Now, the 2nd bit which will take the value from
> convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in
> your new bpc value of 10.
> 
> Can you double-check that this code in DPU1 is actually valid?  I assume you
> have tested this panel at least and it is working (worthy mention in the cover
> letter?), this just seems like yet another mistake in the original bindings
> (though the register always had a matching value with downstream on 8 BPC panels
> for me).

Indeed. msm-4.14 explicitly names that single-bit field as
'input_10_bits'. The block is supposed to support bpc of 8, 10 and 12.
This bit should only be set for bpc=10.

Marijn, thanks for catching it!

We should start rewriting DPU register accessors to use generated
accessors. At least it will clearly show if the field is a flag or a
field which has some values. With the current code it is impossible to
notice the difference.

> 
> > @@ -1779,7 +1774,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
> >  	drm_dsc_set_const_params(dsc);
> >  	drm_dsc_set_rc_buf_thresh(dsc);
> >  
> > -	/* handle only bpp = bpc = 8, pre-SCR panels */
> > +	/* handle only pre-SCR panels */
> >  	ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
> 
> Good catch - this comment sounds like it's documenting a limitation of
> this helper function, but the function does not have such limitations...
> rc_parameters_pre_scr has values for all these combinations.

I think the =8 part is a leftover of the old, pre-helper code.

> 
> - Marijn
> 
> >  	if (ret) {
> >  		DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters\n");
> > -- 
> > 2.48.1
> > 

-- 
With best wishes
Dmitry

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Konrad Dybcio]

On 8.02.2025 11:09 PM, Marijn Suijten wrote:
> On 2025-02-03 21:14:26, Danila Tikhonov wrote:
>> From: Eugene Lepshy <fekz115@gmail.com>
>>
>> DRM DSC helper has parameters for various bpc values ​​other than 8:
> 
> Weird zero-width \u200b spaces here between "values" and "other", please delete
> those.
> 
>> (8/10/12/14/16).
>>
>> Remove this guard.
>>
>> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
>> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> 
> Should this patch elaborate that those "DRM DSC helper" don't have any
> additional guarding for the values you mention either, i.e. passing 9 or 11 or
>> 16 don't seem to be checked anywhere else either?
> 
> And your title might have space to spell out "Bits Per Component" entirely.
> 
>> ---
>>  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 007311c21fda..d182af7bbb81 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -1767,11 +1767,6 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
>>  		return -EINVAL;
>>  	}
>>  
>> -	if (dsc->bits_per_component != 8) {
>> -		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
>> -		return -EOPNOTSUPP;
>> -	}
>> -
>>  	dsc->simple_422 = 0;
>>  	dsc->convert_rgb = 1;
>>  	dsc->vbr_enable = 0;
> 
> This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in
> dpu_hw_dsc_config(), which has:
> 
> 	data |= (dsc->line_buf_depth << 3);
> 	data |= (dsc->simple_422 << 2);
> 	data |= (dsc->convert_rgb << 1);
> 	data |= dsc->bits_per_component;
> 
> The original value of `8` would overlap with the lowest bit of line_buf_depth
> (4th bit in `data`).  Now, the 2nd bit which will take the value from
> convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in
> your new bpc value of 10.
> 
> Can you double-check that this code in DPU1 is actually valid?  I assume you
> have tested this panel at least and it is working (worthy mention in the cover
> letter?), this just seems like yet another mistake in the original bindings
> (though the register always had a matching value with downstream on 8 BPC panels
> for me).

It seems like the lowest bit should be set iff the input is 10bpc, the
current situation where our '8' bleeds into the following (correctly named
fields) is bad.

Konrad

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Konrad Dybcio]

On 10.02.2025 6:10 PM, Konrad Dybcio wrote:
> On 8.02.2025 11:09 PM, Marijn Suijten wrote:
>> On 2025-02-03 21:14:26, Danila Tikhonov wrote:
>>> From: Eugene Lepshy <fekz115@gmail.com>
>>>
>>> DRM DSC helper has parameters for various bpc values ​​other than 8:
>>
>> Weird zero-width \u200b spaces here between "values" and "other", please delete
>> those.
>>
>>> (8/10/12/14/16).
>>>
>>> Remove this guard.
>>>
>>> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
>>> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
>>
>> Should this patch elaborate that those "DRM DSC helper" don't have any
>> additional guarding for the values you mention either, i.e. passing 9 or 11 or
>>> 16 don't seem to be checked anywhere else either?
>>
>> And your title might have space to spell out "Bits Per Component" entirely.
>>
>>> ---
>>>  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 007311c21fda..d182af7bbb81 100644
>>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>>> @@ -1767,11 +1767,6 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
>>>  		return -EINVAL;
>>>  	}
>>>  
>>> -	if (dsc->bits_per_component != 8) {
>>> -		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
>>> -		return -EOPNOTSUPP;
>>> -	}
>>> -
>>>  	dsc->simple_422 = 0;
>>>  	dsc->convert_rgb = 1;
>>>  	dsc->vbr_enable = 0;
>>
>> This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in
>> dpu_hw_dsc_config(), which has:
>>
>> 	data |= (dsc->line_buf_depth << 3);
>> 	data |= (dsc->simple_422 << 2);
>> 	data |= (dsc->convert_rgb << 1);
>> 	data |= dsc->bits_per_component;
>>
>> The original value of `8` would overlap with the lowest bit of line_buf_depth
>> (4th bit in `data`).  Now, the 2nd bit which will take the value from
>> convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in
>> your new bpc value of 10.
>>
>> Can you double-check that this code in DPU1 is actually valid?  I assume you
>> have tested this panel at least and it is working (worthy mention in the cover
>> letter?), this just seems like yet another mistake in the original bindings
>> (though the register always had a matching value with downstream on 8 BPC panels
>> for me).
> 
> It seems like the lowest bit should be set iff the input is 10bpc, the
> current situation where our '8' bleeds into the following (correctly named
> fields) is bad.

See also

https://github.com/Wikidepia/kernel_xiaomi_santoni-4.9/blob/master/drivers/gpu/drm/msm/sde/sde_hw_dsc.c#L67-L80

Konrad

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Marijn Suijten]

On 2025-02-10 18:13:58, Konrad Dybcio wrote:
> On 10.02.2025 6:10 PM, Konrad Dybcio wrote:
> > On 8.02.2025 11:09 PM, Marijn Suijten wrote:
> >> On 2025-02-03 21:14:26, Danila Tikhonov wrote:
...
> >>>  	dsc->simple_422 = 0;
> >>>  	dsc->convert_rgb = 1;
> >>>  	dsc->vbr_enable = 0;
> >>
> >> This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in
> >> dpu_hw_dsc_config(), which has:
> >>
> >> 	data |= (dsc->line_buf_depth << 3);
> >> 	data |= (dsc->simple_422 << 2);
> >> 	data |= (dsc->convert_rgb << 1);
> >> 	data |= dsc->bits_per_component;
> >>
> >> The original value of `8` would overlap with the lowest bit of line_buf_depth
> >> (4th bit in `data`).  Now, the 2nd bit which will take the value from
> >> convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in
> >> your new bpc value of 10.
> >>
> >> Can you double-check that this code in DPU1 is actually valid?  I assume you
> >> have tested this panel at least and it is working (worthy mention in the cover
> >> letter?), this just seems like yet another mistake in the original bindings
> >> (though the register always had a matching value with downstream on 8 BPC panels
> >> for me).
> > 
> > It seems like the lowest bit should be set iff the input is 10bpc, the
> > current situation where our '8' bleeds into the following (correctly named
> > fields) is bad.
> 
> See also
> 
> https://github.com/Wikidepia/kernel_xiaomi_santoni-4.9/blob/master/drivers/gpu/drm/msm/sde/sde_hw_dsc.c#L67-L80

Correct, this is also what Dmitry already replied on Sunday and what I
formulated into a patch earlier today (now sent), which I hope you can ack.

- Marijn

> 
> Konrad

Re: [PATCH 4/4] arm64: dts: qcom: sm7325-nothing-spacewar: Enable panel and GPU

[Konrad Dybcio]

On 3.02.2025 7:14 PM, Danila Tikhonov wrote:
> From: Eugene Lepshy <fekz115@gmail.com>
> 
> Enable the Adreno GPU and configure the Visionox RM692E5 panel.
> 
> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> Co-developed-by: Danila Tikhonov <danila@jiaxyga.com>
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
> Note:
> Depends on https://lore.kernel.org/linux-arm-msm/20250122-dpu-111-topology-v2-1-505e95964af9@somainline.org/
> ---
>  .../boot/dts/qcom/sm7325-nothing-spacewar.dts | 53 ++++++++++++++++++-
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> index a5cda478bd78..cda317b49d5c 100644
> --- a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> +++ b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> @@ -52,6 +52,8 @@ framebuffer0: framebuffer@e1000000 {
>  			stride = <(1080 * 4)>;
>  			format = "a8r8g8b8";
>  
> +			display = <&panel0>;

This is allowed by bindings but doesn't seem to do anything

> +
>  			clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
>  				 <&dispcc DISP_CC_MDSS_MDP_CLK>,
>  				 <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
> @@ -757,6 +759,10 @@ &gpi_dma1 {
>  	status = "okay";
>  };
>  
> +&gpu {
> +	status = "okay";
> +};
> +
>  &gpu_zap_shader {
>  	firmware-name = "qcom/sm7325/nothing/spacewar/a660_zap.mbn";
>  };
> @@ -823,15 +829,44 @@ &ipa {
>  	status = "okay";
>  };
>  
> -/* MDSS remains disabled until the panel driver is present. */
> +&mdss {
> +	status = "okay";
> +};
> +
>  &mdss_dsi {
>  	vdda-supply = <&vdd_a_dsi_0_1p2>;
> +	status = "okay";
>  
> -	/* Visionox RM692E5 panel */
> +	panel0: panel@0 {

Is there going to be a panel1, too? ;)

Please drop the 0

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 4/4] arm64: dts: qcom: sm7325-nothing-spacewar: Enable panel and GPU

[Luca Weiss]

On dinsdag 11 februari 2025 14:31:14 Midden-Europese standaardtijd Konrad Dybcio wrote:
> On 3.02.2025 7:14 PM, Danila Tikhonov wrote:
> > From: Eugene Lepshy <fekz115@gmail.com>
> > 
> > Enable the Adreno GPU and configure the Visionox RM692E5 panel.
> > 
> > Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> > Co-developed-by: Danila Tikhonov <danila@jiaxyga.com>
> > Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> > ---
> > Note:
> > Depends on https://lore.kernel.org/linux-arm-msm/20250122-dpu-111-topology-v2-1-505e95964af9@somainline.org/
> > ---
> >  .../boot/dts/qcom/sm7325-nothing-spacewar.dts | 53 ++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> > index a5cda478bd78..cda317b49d5c 100644
> > --- a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> > +++ b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> > @@ -52,6 +52,8 @@ framebuffer0: framebuffer@e1000000 {
> >  			stride = <(1080 * 4)>;
> >  			format = "a8r8g8b8";
> >  
> > +			display = <&panel0>;
> 
> This is allowed by bindings but doesn't seem to do anything

I believe this exposes display dimensions to user space also via simpledrm.

At least I was using something like this before panel support to get phosh
to scale properly on postmarketOS.

Regards
Luca

> 
> > +
> >  			clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> >  				 <&dispcc DISP_CC_MDSS_MDP_CLK>,
> >  				 <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
> > @@ -757,6 +759,10 @@ &gpi_dma1 {
> >  	status = "okay";
> >  };
> >  
> > +&gpu {
> > +	status = "okay";
> > +};
> > +
> >  &gpu_zap_shader {
> >  	firmware-name = "qcom/sm7325/nothing/spacewar/a660_zap.mbn";
> >  };
> > @@ -823,15 +829,44 @@ &ipa {
> >  	status = "okay";
> >  };
> >  
> > -/* MDSS remains disabled until the panel driver is present. */
> > +&mdss {
> > +	status = "okay";
> > +};
> > +
> >  &mdss_dsi {
> >  	vdda-supply = <&vdd_a_dsi_0_1p2>;
> > +	status = "okay";
> >  
> > -	/* Visionox RM692E5 panel */
> > +	panel0: panel@0 {
> 
> Is there going to be a panel1, too? ;)
> 
> Please drop the 0
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> Konrad
> 

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Danila Tikhonov]

On 2/9/25 01:09, Marijn Suijten wrote:
> On 2025-02-03 21:14:26, Danila Tikhonov wrote:
>> From: Eugene Lepshy <fekz115@gmail.com>
>>
>> DRM DSC helper has parameters for various bpc values ​​other than 8:
> Weird zero-width \u200b spaces here between "values" and "other", please delete
> those.
Thanks, I will fix it in the next version.
>> (8/10/12/14/16).
>>
>> Remove this guard.
>>
>> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
>> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> Should this patch elaborate that those "DRM DSC helper" don't have any
> additional guarding for the values you mention either, i.e. passing 9 or 11 or
>> 16 don't seem to be checked anywhere else either?
There are no other bpc checks, you are right. But to be honest I don't
really see any sense in this. Anyway, if you still want us to leave the
current guard and just extend it with new values ​​(for example via
switch case) - let me know.
> And your title might have space to spell out "Bits Per Component" entirely.
I'll fix that too.
>> ---
>>   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 007311c21fda..d182af7bbb81 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -1767,11 +1767,6 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
>>   		return -EINVAL;
>>   	}
>>   
>> -	if (dsc->bits_per_component != 8) {
>> -		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
>> -		return -EOPNOTSUPP;
>> -	}
>> -
>>   	dsc->simple_422 = 0;
>>   	dsc->convert_rgb = 1;
>>   	dsc->vbr_enable = 0;
> This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in
> dpu_hw_dsc_config(), which has:
>
> 	data |= (dsc->line_buf_depth << 3);
> 	data |= (dsc->simple_422 << 2);
> 	data |= (dsc->convert_rgb << 1);
> 	data |= dsc->bits_per_component;
>
> The original value of `8` would overlap with the lowest bit of line_buf_depth
> (4th bit in `data`).  Now, the 2nd bit which will take the value from
> convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in
> your new bpc value of 10.
>
> Can you double-check that this code in DPU1 is actually valid?  I assume you
> have tested this panel at least and it is working (worthy mention in the cover
> letter?), this just seems like yet another mistake in the original bindings
> (though the register always had a matching value with downstream on 8 BPC panels
> for me).

Of course I have tested the panel and it works, I just thought it would
be obvious. We also have tested sm7150-xiaomi-courbet, sm8450-xiaomi-cupid
and sm8475-nothing-pong, which already have bpp = bpc = 10 panels and
with some hack it also work without any changes to the DRM.

>> @@ -1779,7 +1774,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
>>   	drm_dsc_set_const_params(dsc);
>>   	drm_dsc_set_rc_buf_thresh(dsc);
>>   
>> -	/* handle only bpp = bpc = 8, pre-SCR panels */
>> +	/* handle only pre-SCR panels */
>>   	ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
> Good catch - this comment sounds like it's documenting a limitation of
> this helper function, but the function does not have such limitations...
> rc_parameters_pre_scr has values for all these combinations.
Maybe we should remove this comment entirely?

Regards,
Danila
> - Marijn
>
>>   	if (ret) {
>>   		DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters\n");
>> -- 
>> 2.48.1
>>

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Dmitry Baryshkov]

On Tue, Feb 11, 2025 at 09:06:19PM +0300, Danila Tikhonov wrote:
> On 2/9/25 01:09, Marijn Suijten wrote:
> > On 2025-02-03 21:14:26, Danila Tikhonov wrote:
> > > From: Eugene Lepshy <fekz115@gmail.com>
> > > 
> > > DRM DSC helper has parameters for various bpc values ​​other than 8:
> > Weird zero-width \u200b spaces here between "values" and "other", please delete
> > those.
> Thanks, I will fix it in the next version.
> > > (8/10/12/14/16).
> > > 
> > > Remove this guard.
> > > 
> > > Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> > > Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> > Should this patch elaborate that those "DRM DSC helper" don't have any
> > additional guarding for the values you mention either, i.e. passing 9 or 11 or
> > > 16 don't seem to be checked anywhere else either?
> There are no other bpc checks, you are right. But to be honest I don't
> really see any sense in this. Anyway, if you still want us to leave the
> current guard and just extend it with new values ​​(for example via
> switch case) - let me know.

Yes, please. Add a caselist and also a note that only 8, 10 and 12 are
valid for DSC 1.1 block. Then whoever stomps upon other bpc value will
have to extend the check, verifying DSC version.

> > And your title might have space to spell out "Bits Per Component" entirely.
> I'll fix that too.
> > > ---
> > >   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 007311c21fda..d182af7bbb81 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > @@ -1767,11 +1767,6 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
> > >   		return -EINVAL;
> > >   	}
> > > -	if (dsc->bits_per_component != 8) {
> > > -		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
> > > -		return -EOPNOTSUPP;
> > > -	}
> > > -
> > >   	dsc->simple_422 = 0;
> > >   	dsc->convert_rgb = 1;
> > >   	dsc->vbr_enable = 0;
> > This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in
> > dpu_hw_dsc_config(), which has:
> > 
> > 	data |= (dsc->line_buf_depth << 3);
> > 	data |= (dsc->simple_422 << 2);
> > 	data |= (dsc->convert_rgb << 1);
> > 	data |= dsc->bits_per_component;
> > 
> > The original value of `8` would overlap with the lowest bit of line_buf_depth
> > (4th bit in `data`).  Now, the 2nd bit which will take the value from
> > convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in
> > your new bpc value of 10.
> > 
> > Can you double-check that this code in DPU1 is actually valid?  I assume you
> > have tested this panel at least and it is working (worthy mention in the cover
> > letter?), this just seems like yet another mistake in the original bindings
> > (though the register always had a matching value with downstream on 8 BPC panels
> > for me).
> 
> Of course I have tested the panel and it works, I just thought it would
> be obvious. We also have tested sm7150-xiaomi-courbet, sm8450-xiaomi-cupid
> and sm8475-nothing-pong, which already have bpp = bpc = 10 panels and
> with some hack it also work without any changes to the DRM.

This is now being fixed by a separate patch.

> 
> > > @@ -1779,7 +1774,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
> > >   	drm_dsc_set_const_params(dsc);
> > >   	drm_dsc_set_rc_buf_thresh(dsc);
> > > -	/* handle only bpp = bpc = 8, pre-SCR panels */
> > > +	/* handle only pre-SCR panels */
> > >   	ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
> > Good catch - this comment sounds like it's documenting a limitation of
> > this helper function, but the function does not have such limitations...
> > rc_parameters_pre_scr has values for all these combinations.
> Maybe we should remove this comment entirely?

No, the pre-SCR comment is fine.

> 
> Regards,
> Danila
> > - Marijn
> > 
> > >   	if (ret) {
> > >   		DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters\n");
> > > -- 
> > > 2.48.1
> > > 

-- 
With best wishes
Dmitry

Re: [PATCH 4/4] arm64: dts: qcom: sm7325-nothing-spacewar: Enable panel and GPU

[Dmitry Baryshkov]

On Tue, Feb 11, 2025 at 02:31:14PM +0100, Konrad Dybcio wrote:
> On 3.02.2025 7:14 PM, Danila Tikhonov wrote:
> > From: Eugene Lepshy <fekz115@gmail.com>
> > 
> > Enable the Adreno GPU and configure the Visionox RM692E5 panel.
> > 
> > Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> > Co-developed-by: Danila Tikhonov <danila@jiaxyga.com>
> > Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> > ---
> > Note:
> > Depends on https://lore.kernel.org/linux-arm-msm/20250122-dpu-111-topology-v2-1-505e95964af9@somainline.org/
> > ---
> >  .../boot/dts/qcom/sm7325-nothing-spacewar.dts | 53 ++++++++++++++++++-
> >  1 file changed, 51 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> > index a5cda478bd78..cda317b49d5c 100644
> > --- a/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> > +++ b/arch/arm64/boot/dts/qcom/sm7325-nothing-spacewar.dts
> > @@ -52,6 +52,8 @@ framebuffer0: framebuffer@e1000000 {
> >  			stride = <(1080 * 4)>;
> >  			format = "a8r8g8b8";
> >  
> > +			display = <&panel0>;
> 
> This is allowed by bindings but doesn't seem to do anything
> 
> > +
> >  			clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> >  				 <&dispcc DISP_CC_MDSS_MDP_CLK>,
> >  				 <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
> > @@ -757,6 +759,10 @@ &gpi_dma1 {
> >  	status = "okay";
> >  };
> >  
> > +&gpu {
> > +	status = "okay";
> > +};
> > +
> >  &gpu_zap_shader {
> >  	firmware-name = "qcom/sm7325/nothing/spacewar/a660_zap.mbn";
> >  };
> > @@ -823,15 +829,44 @@ &ipa {
> >  	status = "okay";
> >  };
> >  
> > -/* MDSS remains disabled until the panel driver is present. */
> > +&mdss {
> > +	status = "okay";
> > +};
> > +
> >  &mdss_dsi {
> >  	vdda-supply = <&vdd_a_dsi_0_1p2>;
> > +	status = "okay";
> >  
> > -	/* Visionox RM692E5 panel */
> > +	panel0: panel@0 {
> 
> Is there going to be a panel1, too? ;)
> 
> Please drop the 0

No, dsi-controller.yaml requires panel@[0-3] and a reg entry.

> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> 
> Konrad

-- 
With best wishes
Dmitry

Re: [PATCH 3/4] drm/msm/dsi: Allow all bpc values

[Marijn Suijten]

On 2025-02-11 21:06:19, Danila Tikhonov wrote:
> On 2/9/25 01:09, Marijn Suijten wrote:
> > On 2025-02-03 21:14:26, Danila Tikhonov wrote:
> >> From: Eugene Lepshy <fekz115@gmail.com>
> >>
> >> DRM DSC helper has parameters for various bpc values ​​other than 8:
> > Weird zero-width \u200b spaces here between "values" and "other", please delete
> > those.
> Thanks, I will fix it in the next version.
> >> (8/10/12/14/16).
> >>
> >> Remove this guard.
> >>
> >> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
> >> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> > Should this patch elaborate that those "DRM DSC helper" don't have any
> > additional guarding for the values you mention either, i.e. passing 9 or 11 or
> >> 16 don't seem to be checked anywhere else either?
> There are no other bpc checks, you are right. But to be honest I don't
> really see any sense in this. Anyway, if you still want us to leave the
> current guard and just extend it with new values ​​(for example via
> switch case) - let me know.

Yes please: extend the guard.  drm_dsc_setup_rc_params() supports 8bbp with bpc
of 8, 10, 12, 14 or 16, which are not all supported by DPU (yet?).  Any other
value should be considered an error.

> > And your title might have space to spell out "Bits Per Component" entirely.
> I'll fix that too.
> >> ---
> >>   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 007311c21fda..d182af7bbb81 100644
> >> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> >> @@ -1767,11 +1767,6 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
> >>   		return -EINVAL;
> >>   	}
> >>   
> >> -	if (dsc->bits_per_component != 8) {
> >> -		DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
> >> -		return -EOPNOTSUPP;
> >> -	}
> >> -
> >>   	dsc->simple_422 = 0;
> >>   	dsc->convert_rgb = 1;
> >>   	dsc->vbr_enable = 0;
> > This seems supicous on the dpu1 side, in the original DSC 1.1 (not 1.2) block in
> > dpu_hw_dsc_config(), which has:
> >
> > 	data |= (dsc->line_buf_depth << 3);
> > 	data |= (dsc->simple_422 << 2);
> > 	data |= (dsc->convert_rgb << 1);
> > 	data |= dsc->bits_per_component;
> >
> > The original value of `8` would overlap with the lowest bit of line_buf_depth
> > (4th bit in `data`).  Now, the 2nd bit which will take the value from
> > convert_rgb, which is already set to 1 above, will overlap with the 2nd bit in
> > your new bpc value of 10.
> >
> > Can you double-check that this code in DPU1 is actually valid?  I assume you
> > have tested this panel at least and it is working (worthy mention in the cover
> > letter?), this just seems like yet another mistake in the original bindings
> > (though the register always had a matching value with downstream on 8 BPC panels
> > for me).
> 
> Of course I have tested the panel and it works, I just thought it would
> be obvious.

It's worth mentioning, also if the timings / framerate are correct or if there's
any hiccups (common on CMD panels).

> We also have tested sm7150-xiaomi-courbet, sm8450-xiaomi-cupid
> and sm8475-nothing-pong, which already have bpp = bpc = 10 panels and
> with some hack it also work without any changes to the DRM.

It's strange because, as mentioned in a followup patch all the bits in bpc=10
overlap with existing bits in the data field except the lowest bit of data
wasn't getting set even though it should according to downstream sources.  Can
you test [1] and confirm whether "correctly" setting the lowest bit in the data
field doesn't break anything?

[1]: https://lore.kernel.org/linux-arm-msm/20250211-dsc-10-bit-v1-1-1c85a9430d9a@somainline.org/

> >> @@ -1779,7 +1774,7 @@ static int dsi_populate_dsc_params(struct msm_dsi_host *msm_host, struct drm_dsc
> >>   	drm_dsc_set_const_params(dsc);
> >>   	drm_dsc_set_rc_buf_thresh(dsc);
> >>   
> >> -	/* handle only bpp = bpc = 8, pre-SCR panels */
> >> +	/* handle only pre-SCR panels */
> >>   	ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
> > Good catch - this comment sounds like it's documenting a limitation of
> > this helper function, but the function does not have such limitations...
> > rc_parameters_pre_scr has values for all these combinations.
> Maybe we should remove this comment entirely?

Maybe it should be reworded instead.  It's clear that the function will handle
pre-SCR panels, that's why this "type" enum variant is passed.  Perhaps it
should instead explain that DPU (in its current implementation) only supports
pre-SCR panels?

- Marijn

> 
> Regards,
> Danila
> > - Marijn
> >
> >>   	if (ret) {
> >>   		DRM_DEV_ERROR(&msm_host->pdev->dev, "could not find DSC RC parameters\n");
> >> -- 
> >> 2.48.1
> >>

Re: [PATCH 4/4] arm64: dts: qcom: sm7325-nothing-spacewar: Enable panel and GPU

[Konrad Dybcio]

On 12.02.2025 12:53 AM, Dmitry Baryshkov wrote:
> On Tue, Feb 11, 2025 at 02:31:14PM +0100, Konrad Dybcio wrote:
>> On 3.02.2025 7:14 PM, Danila Tikhonov wrote:
>>> From: Eugene Lepshy <fekz115@gmail.com>
>>>
>>> Enable the Adreno GPU and configure the Visionox RM692E5 panel.
>>>
>>> Signed-off-by: Eugene Lepshy <fekz115@gmail.com>
>>> Co-developed-by: Danila Tikhonov <danila@jiaxyga.com>
>>> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
>>> ---

[...]

>>> +
>>>  &mdss_dsi {
>>>  	vdda-supply = <&vdd_a_dsi_0_1p2>;
>>> +	status = "okay";
>>>  
>>> -	/* Visionox RM692E5 panel */
>>> +	panel0: panel@0 {
>>
>> Is there going to be a panel1, too? ;)
>>
>> Please drop the 0
> 
> No, dsi-controller.yaml requires panel@[0-3] and a reg entry.

I meant the zero in the label

Konrad