[PATCH 0/5] rockchip: Add Powkiddy RK2023

[PATCH 0/5] rockchip: Add Powkiddy RK2023

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the Powkiddy RK2023, which is extremely similar to
existing devices from Anbernic.

Chris Morgan (5):
  dt-bindings: display: panel: Update NewVision NV3051D compatibles
  drm/panel: nv3051d: Add Powkiddy RK2023 Panel Support
  clk: rockchip: rk3568: Add PLL rate for 115.2MHz
  dt-bindings: arm: rockchip: Add Powkiddy RK2023
  arm64: dts: rockchip: add Powkiddy RK2023

 .../devicetree/bindings/arm/rockchip.yaml     |   5 +
 .../display/panel/newvision,nv3051d.yaml      |   2 +-
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../dts/rockchip/rk3566-powkiddy-rk2023.dts   | 161 ++++++++++++++++++
 drivers/clk/rockchip/clk-rk3568.c             |   1 +
 .../gpu/drm/panel/panel-newvision-nv3051d.c   |  56 ++++--
 6 files changed, 214 insertions(+), 12 deletions(-)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dts

-- 
2.34.1

[PATCH 1/5] dt-bindings: display: panel: Update NewVision NV3051D compatibles

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Update the NewVision NV3051D compatible strings by adding a new panel,
the powkiddy,rk2023-panel, and removing another entry, the
anbernic,rg353v-panel. The rg353v-panel is exactly identical to the
rg353p-panel and is not currently in use by any existing device tree.
The rk2023-panel is similar to the rg353p-panel but has slightly
different timings.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../devicetree/bindings/display/panel/newvision,nv3051d.yaml    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
index cce775a87f87..7a634fbc465e 100644
--- a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
+++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
@@ -21,7 +21,7 @@ properties:
       - enum:
           - anbernic,rg351v-panel
           - anbernic,rg353p-panel
-          - anbernic,rg353v-panel
+          - powkiddy,rk2023-panel
       - const: newvision,nv3051d
 
   reg: true
-- 
2.34.1

[PATCH 2/5] drm/panel: nv3051d: Add Powkiddy RK2023 Panel Support

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Refactor the driver to add support for the powkiddy,rk2023-panel
panel. This panel is extremely similar to the rg353p-panel but
requires a smaller vertical back porch and isn't as tolerant of
higher speeds.

Tested on my RG351V, RG353P, RG353V, and RK2023.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../gpu/drm/panel/panel-newvision-nv3051d.c   | 56 +++++++++++++++----
 1 file changed, 45 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
index 79de6c886292..d24c51503d68 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
@@ -28,6 +28,7 @@ struct nv3051d_panel_info {
 	unsigned int num_modes;
 	u16 width_mm, height_mm;
 	u32 bus_flags;
+	u32 mode_flags;
 };
 
 struct panel_nv3051d {
@@ -385,15 +386,7 @@ static int panel_nv3051d_probe(struct mipi_dsi_device *dsi)
 
 	dsi->lanes = 4;
 	dsi->format = MIPI_DSI_FMT_RGB888;
-	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
-			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
-
-	/*
-	 * The panel in the RG351V is identical to the 353P, except it
-	 * requires MIPI_DSI_CLOCK_NON_CONTINUOUS to operate correctly.
-	 */
-	if (of_device_is_compatible(dev->of_node, "anbernic,rg351v-panel"))
-		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
+	dsi->mode_flags = ctx->panel_info->mode_flags;
 
 	drm_panel_init(&ctx->panel, &dsi->dev, &panel_nv3051d_funcs,
 		       DRM_MODE_CONNECTOR_DSI);
@@ -481,18 +474,59 @@ static const struct drm_display_mode nv3051d_rgxx3_modes[] = {
 	},
 };
 
-static const struct nv3051d_panel_info nv3051d_rgxx3_info = {
+static const struct drm_display_mode nv3051d_rk2023_modes[] = {
+	{
+		.hdisplay       = 640,
+		.hsync_start    = 640 + 40,
+		.hsync_end      = 640 + 40 + 2,
+		.htotal         = 640 + 40 + 2 + 80,
+		.vdisplay       = 480,
+		.vsync_start    = 480 + 18,
+		.vsync_end      = 480 + 18 + 2,
+		.vtotal         = 480 + 18 + 2 + 4,
+		.clock          = 24150,
+		.flags          = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
+	},
+};
+
+static const struct nv3051d_panel_info nv3051d_rg351v_info = {
 	.display_modes = nv3051d_rgxx3_modes,
 	.num_modes = ARRAY_SIZE(nv3051d_rgxx3_modes),
 	.width_mm = 70,
 	.height_mm = 57,
 	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET |
+		      MIPI_DSI_CLOCK_NON_CONTINUOUS,
+};
+
+static const struct nv3051d_panel_info nv3051d_rg353p_info = {
+	.display_modes = nv3051d_rgxx3_modes,
+	.num_modes = ARRAY_SIZE(nv3051d_rgxx3_modes),
+	.width_mm = 70,
+	.height_mm = 57,
+	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET,
+};
+
+static const struct nv3051d_panel_info nv3051d_rk2023_info = {
+	.display_modes = nv3051d_rk2023_modes,
+	.num_modes = ARRAY_SIZE(nv3051d_rk2023_modes),
+	.width_mm = 70,
+	.height_mm = 57,
+	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
+	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET,
 };
 
 static const struct of_device_id newvision_nv3051d_of_match[] = {
-	{ .compatible = "newvision,nv3051d", .data = &nv3051d_rgxx3_info },
+	{ .compatible = "anbernic,rg351v-panel", .data = &nv3051d_rg351v_info },
+	{ .compatible = "anbernic,rg353p-panel", .data = &nv3051d_rg353p_info },
+	{ .compatible = "powkiddy,rk2023-panel", .data = &nv3051d_rk2023_info },
 	{ /* sentinel */ }
 };
+
 MODULE_DEVICE_TABLE(of, newvision_nv3051d_of_match);
 
 static struct mipi_dsi_driver newvision_nv3051d_driver = {
-- 
2.34.1

[PATCH 3/5] clk: rockchip: rk3568: Add PLL rate for 115.2MHz

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add support for a PLL rate of 115.2MHz so that the Powkiddy RK2023 panel
can run at a requested 60hz (59.99, close enough).

I have confirmed this rate fits with all the constraints
listed in the TRM for the VPLL (as an integer PLL) in Part 1 "Chapter
2 Clock & Reset Unit (CRU)."

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/clk/rockchip/clk-rk3568.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/rockchip/clk-rk3568.c b/drivers/clk/rockchip/clk-rk3568.c
index db713e1526cd..bfbcbb744327 100644
--- a/drivers/clk/rockchip/clk-rk3568.c
+++ b/drivers/clk/rockchip/clk-rk3568.c
@@ -79,6 +79,7 @@ static struct rockchip_pll_rate_table rk3568_pll_rates[] = {
 	RK3036_PLL_RATE(148500000, 1, 99, 4, 4, 1, 0),
 	RK3036_PLL_RATE(135000000, 2, 45, 4, 1, 1, 0),
 	RK3036_PLL_RATE(119000000, 3, 119, 4, 2, 1, 0),
+	RK3036_PLL_RATE(115200000, 1, 24, 5, 1, 1, 0),
 	RK3036_PLL_RATE(108000000, 2, 45, 5, 1, 1, 0),
 	RK3036_PLL_RATE(101000000, 1, 101, 6, 4, 1, 0),
 	RK3036_PLL_RATE(100000000, 1, 150, 6, 6, 1, 0),
-- 
2.34.1

[PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
powered by the Rockchip RK3566 SoC.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
index a349bf4da6bc..a6612185a7ff 100644
--- a/Documentation/devicetree/bindings/arm/rockchip.yaml
+++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
@@ -674,6 +674,11 @@ properties:
           - const: powkiddy,rgb30
           - const: rockchip,rk3566
 
+      - description: Powkiddy RK2023
+        items:
+          - const: powkiddy,rk2023
+          - const: rockchip,rk3566
+
       - description: Radxa Compute Module 3(CM3)
         items:
           - enum:
-- 
2.34.1

[PATCH 5/5] arm64: dts: rockchip: add Powkiddy RK2023

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the Powkiddy RK2023. The Powkiddy RK2023 is a handheld
gaming device with a 3.5 inch screen powered by the Rockchip RK3566
SoC. The device is almost identical to the Anbernic RG353P except it
lacks eMMC, a function button, a touch screen, no UART headers on the
board, and the panel has slightly different timings.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../dts/rockchip/rk3566-powkiddy-rk2023.dts   | 161 ++++++++++++++++++
 2 files changed, 162 insertions(+)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dts

diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
index 3f01b429a3aa..9ef64cfb8392 100644
--- a/arch/arm64/boot/dts/rockchip/Makefile
+++ b/arch/arm64/boot/dts/rockchip/Makefile
@@ -78,6 +78,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg503.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.1.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-powkiddy-rgb30.dtb
+dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-powkiddy-rk2023.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-b.dtb
 dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-radxa-cm3-io.dtb
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dts b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dts
new file mode 100644
index 000000000000..5740412f6b2b
--- /dev/null
+++ b/arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dts
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/linux-event-codes.h>
+#include <dt-bindings/pinctrl/rockchip.h>
+#include "rk3566-anbernic-rg353x.dtsi"
+
+/ {
+	model = "RK2023";
+	compatible = "powkiddy,rk2023", "rockchip,rk3566";
+
+	aliases {
+		mmc1 = &sdmmc0;
+		mmc2 = &sdmmc1;
+		mmc3 = &sdmmc2;
+	};
+
+	battery: battery {
+		compatible = "simple-battery";
+		charge-full-design-microamp-hours = <3151000>;
+		charge-term-current-microamp = <300000>;
+		constant-charge-current-max-microamp = <2000000>;
+		constant-charge-voltage-max-microvolt = <4250000>;
+		factory-internal-resistance-micro-ohms = <117000>;
+		voltage-max-design-microvolt = <4172000>;
+		voltage-min-design-microvolt = <3400000>;
+
+		ocv-capacity-celsius = <20>;
+		ocv-capacity-table-0 =  <4172000 100>, <4092000 95>, <4035000 90>, <3990000 85>,
+					<3939000 80>, <3895000 75>, <3852000 70>, <3807000 65>,
+					<3762000 60>, <3713000 55>, <3672000 50>, <3647000 45>,
+					<3629000 40>, <3613000 35>, <3598000 30>, <3578000 25>,
+					<3550000 20>, <3519000 15>, <3479000 10>, <3438000 5>,
+					<3400000 0>;
+	};
+
+	/* Channels reversed for headphones. */
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,name = "rk817_int";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,hp-det-gpio = <&gpio4 RK_PC6 GPIO_ACTIVE_HIGH>;
+		simple-audio-card,mclk-fs = <256>;
+		simple-audio-card,widgets =
+			"Microphone", "Mic Jack",
+			"Headphone", "Headphones",
+			"Speaker", "Internal Speakers";
+		simple-audio-card,routing =
+			"MICL", "Mic Jack",
+			"Headphones", "HPOL",
+			"Headphones", "HPOR",
+			"Internal Speakers", "SPKO";
+
+		simple-audio-card,codec {
+			sound-dai = <&rk817>;
+		};
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s1_8ch>;
+		};
+	};
+
+};
+
+/delete-node/ &adc_keys;
+
+&chosen {
+	/delete-property/ stdout-path;
+};
+
+&cru {
+	assigned-clocks = <&pmucru CLK_RTC_32K>, <&cru PLL_GPLL>,
+			  <&pmucru PLL_PPLL>, <&cru PLL_VPLL>;
+	assigned-clock-rates = <32768>, <1200000000>,
+			  <200000000>, <115200000>;
+};
+
+&gpio_keys_control {
+	button-r1 {
+		gpios = <&gpio3 RK_PB3 GPIO_ACTIVE_LOW>;
+		label = "TR";
+		linux,code = <BTN_TR>;
+	};
+
+	button-r2 {
+		gpios = <&gpio3 RK_PB4 GPIO_ACTIVE_LOW>;
+		label = "TR2";
+		linux,code = <BTN_TR2>;
+	};
+};
+
+/delete-node/ &{/i2c@fdd40000/regulator@40};
+
+&i2c0 {
+	vdd_cpu: regulator@1c {
+		compatible = "tcs,tcs4525";
+		reg = <0x1c>;
+		fcs,suspend-voltage-selector = <1>;
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-min-microvolt = <712500>;
+		regulator-max-microvolt = <1390000>;
+		regulator-name = "vdd_cpu";
+		regulator-ramp-delay = <2300>;
+		vin-supply = <&vcc_sys>;
+		regulator-state-mem {
+			regulator-off-in-suspend;
+		};
+	};
+};
+
+/*
+ * Device has 2 controllable LEDs (a red and a green) along with a red
+ * LED that cannot be controlled and is always on when device is
+ * powered.
+ */
+
+/delete-node/ &{/pwm-leds/led-2};
+
+&leds {
+	green_led: led-0 {
+		color = <LED_COLOR_ID_GREEN>;
+		function = LED_FUNCTION_STATUS;
+		max-brightness = <255>;
+		pwms = <&pwm6 0 25000 0>;
+	};
+
+	red_led: led-1 {
+		color = <LED_COLOR_ID_RED>;
+		function = LED_FUNCTION_CHARGING;
+		max-brightness = <255>;
+		pwms = <&pwm7 0 25000 0>;
+	};
+
+};
+
+&panel {
+	compatible = "powkiddy,rk2023-panel", "newvision,nv3051d";
+};
+
+&pwm5 {
+	status = "disabled";
+};
+
+&rk817 {
+	rk817_charger: charger {
+		monitored-battery = <&battery>;
+		rockchip,resistor-sense-micro-ohms = <10000>;
+		rockchip,sleep-enter-current-microamp = <300000>;
+		rockchip,sleep-filter-current-microamp = <100000>;
+	};
+};
+
+/* There is no UART header visible on the board for this device. */
+&uart2 {
+	status = "disabled";
+};
+
+/delete-node/ &vibrator;
-- 
2.34.1

Re: (subset) [PATCH 0/5] rockchip: Add Powkiddy RK2023

[Heiko Stuebner]

On Wed, 18 Oct 2023 11:18:43 -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add support for the Powkiddy RK2023, which is extremely similar to
> existing devices from Anbernic.
> 
> Chris Morgan (5):
>   dt-bindings: display: panel: Update NewVision NV3051D compatibles
>   drm/panel: nv3051d: Add Powkiddy RK2023 Panel Support
>   clk: rockchip: rk3568: Add PLL rate for 115.2MHz
>   dt-bindings: arm: rockchip: Add Powkiddy RK2023
>   arm64: dts: rockchip: add Powkiddy RK2023
> 
> [...]

Applied, thanks!

[3/5] clk: rockchip: rk3568: Add PLL rate for 115.2MHz
      commit: ccf59682a0287b81015dc1939203fac70b818c6b

I've gone forward and grabbed the PLL rate already, so it
can go together with the other rate addition from the fixes
series.

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

[Krzysztof Kozlowski]

On 18/10/2023 18:18, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> powered by the Rockchip RK3566 SoC.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> index a349bf4da6bc..a6612185a7ff 100644
> --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> @@ -674,6 +674,11 @@ properties:
>            - const: powkiddy,rgb30
>            - const: rockchip,rk3566
>  
> +      - description: Powkiddy RK2023
> +        items:
> +          - const: powkiddy,rk2023

This cuold be just enum in previous entry :/ but I remember we talked
about this once with Heiko.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

> +          - const: rockchip,rk3566



Best regards,
Krzysztof

Re: [PATCH 1/5] dt-bindings: display: panel: Update NewVision NV3051D compatibles

[Krzysztof Kozlowski]

On 18/10/2023 18:18, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Update the NewVision NV3051D compatible strings by adding a new panel,
> the powkiddy,rk2023-panel, and removing another entry, the
> anbernic,rg353v-panel. The rg353v-panel is exactly identical to the
> rg353p-panel and is not currently in use by any existing device tree.
> The rk2023-panel is similar to the rg353p-panel but has slightly
> different timings.

This still does not explain me why do you want to remove old panel.



Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

[Heiko Stübner]

Am Donnerstag, 19. Oktober 2023, 11:21:47 CEST schrieb Krzysztof Kozlowski:
> On 18/10/2023 18:18, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > powered by the Rockchip RK3566 SoC.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > index a349bf4da6bc..a6612185a7ff 100644
> > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > @@ -674,6 +674,11 @@ properties:
> >            - const: powkiddy,rgb30
> >            - const: rockchip,rk3566
> >  
> > +      - description: Powkiddy RK2023
> > +        items:
> > +          - const: powkiddy,rk2023
> 
> This cuold be just enum in previous entry :/ but I remember we talked
> about this once with Heiko.

Keeping similar devices together is perfectly fine. Like we do for example
with the Rock PI 4A/4A+/B/B+/C family directly below.

The powkiddy,rk2023 really looks like very similar to the rgb30, so
could do something similar.


The variant I don't like is having one big enum for _all_  boards using
the same soc.


Heiko

> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> > +          - const: rockchip,rk3566
> 
> 
> 
> Best regards,
> Krzysztof
> 
> 

Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

[Chris Morgan]

On Thu, Oct 19, 2023 at 11:21:47AM +0200, Krzysztof Kozlowski wrote:
> On 18/10/2023 18:18, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > powered by the Rockchip RK3566 SoC.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > index a349bf4da6bc..a6612185a7ff 100644
> > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > @@ -674,6 +674,11 @@ properties:
> >            - const: powkiddy,rgb30
> >            - const: rockchip,rk3566
> >  
> > +      - description: Powkiddy RK2023
> > +        items:
> > +          - const: powkiddy,rk2023
> 
> This cuold be just enum in previous entry :/ but I remember we talked
> about this once with Heiko.

For hardware that requires a different device tree, is that possible?
While most of the devices I've worked on for the RK3566 series are very
similar for the moment only 1 is identical (the RG353P and the RG353M)
and can use the same device tree.

Also I have one more Powkiddy device to send probably in the next week
or two, then I'll be breaking for a while from new devices. :-)

Thank you,
Chris

> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> > +          - const: rockchip,rk3566
> 
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/5] dt-bindings: display: panel: Update NewVision NV3051D compatibles

[Chris Morgan]

On Thu, Oct 19, 2023 at 11:22:19AM +0200, Krzysztof Kozlowski wrote:
> On 18/10/2023 18:18, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Update the NewVision NV3051D compatible strings by adding a new panel,
> > the powkiddy,rk2023-panel, and removing another entry, the
> > anbernic,rg353v-panel. The rg353v-panel is exactly identical to the
> > rg353p-panel and is not currently in use by any existing device tree.
> > The rk2023-panel is similar to the rg353p-panel but has slightly
> > different timings.
> 
> This still does not explain me why do you want to remove old panel.

When I originally wrote the driver I only had one piece of hardware
and I set the compatible string in the driver as newvision,nv3051d.
Unfortunately since then I've found 2 more devices in use that are
*just* different enough to require the driver to do things a bit
differently. In the case of the anbernic,rg351v-panel I need to
enable a new DSI flag; in the case of the powkiddy,rk2023-panel I need
to decrease the vertical back porch and drop the higher frequency
timings.

The best way to accomplish this was to change the strategy from having
a single binding in the driver of newvision,nv3051d to a binding for
each distinct hardware where the differences apply. Note that I've
looked at querying the DSI panel ID, but for each device the value
is identical (so it can't be used to differentiate the hardware sadly).
So the driver now has 3 different compatible strings. I could in this
case add a 4th compatible string of anbernic,rg353v-panel but it would
be identical to anbernic,rg353p-panel. For the moment we are using
anbernic,rg353p-panel everywhere (including the rg353v), so it makes
sense to drop this unused value while we can, at least to me.

Let me know if you have any more questions, thank you.
Chris

> 
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 2/5] drm/panel: nv3051d: Add Powkiddy RK2023 Panel Support

[Jessica Zhang]

On 10/18/2023 9:18 AM, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Refactor the driver to add support for the powkiddy,rk2023-panel
> panel. This panel is extremely similar to the rg353p-panel but
> requires a smaller vertical back porch and isn't as tolerant of
> higher speeds.
> 
> Tested on my RG351V, RG353P, RG353V, and RK2023.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>

Hi Chris,

Thanks for the patch. Just have a minor question below.

> ---
>   .../gpu/drm/panel/panel-newvision-nv3051d.c   | 56 +++++++++++++++----
>   1 file changed, 45 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> index 79de6c886292..d24c51503d68 100644
> --- a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> @@ -28,6 +28,7 @@ struct nv3051d_panel_info {
>   	unsigned int num_modes;
>   	u16 width_mm, height_mm;
>   	u32 bus_flags;
> +	u32 mode_flags;
>   };
>   
>   struct panel_nv3051d {
> @@ -385,15 +386,7 @@ static int panel_nv3051d_probe(struct mipi_dsi_device *dsi)
>   
>   	dsi->lanes = 4;
>   	dsi->format = MIPI_DSI_FMT_RGB888;
> -	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> -			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
> -
> -	/*
> -	 * The panel in the RG351V is identical to the 353P, except it
> -	 * requires MIPI_DSI_CLOCK_NON_CONTINUOUS to operate correctly.
> -	 */
> -	if (of_device_is_compatible(dev->of_node, "anbernic,rg351v-panel"))
> -		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
> +	dsi->mode_flags = ctx->panel_info->mode_flags;
>   
>   	drm_panel_init(&ctx->panel, &dsi->dev, &panel_nv3051d_funcs,
>   		       DRM_MODE_CONNECTOR_DSI);
> @@ -481,18 +474,59 @@ static const struct drm_display_mode nv3051d_rgxx3_modes[] = {
>   	},
>   };
>   
> -static const struct nv3051d_panel_info nv3051d_rgxx3_info = {
> +static const struct drm_display_mode nv3051d_rk2023_modes[] = {
> +	{
> +		.hdisplay       = 640,
> +		.hsync_start    = 640 + 40,
> +		.hsync_end      = 640 + 40 + 2,
> +		.htotal         = 640 + 40 + 2 + 80,
> +		.vdisplay       = 480,
> +		.vsync_start    = 480 + 18,
> +		.vsync_end      = 480 + 18 + 2,
> +		.vtotal         = 480 + 18 + 2 + 4,
> +		.clock          = 24150,
> +		.flags          = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +	},
> +};
> +
> +static const struct nv3051d_panel_info nv3051d_rg351v_info = {
>   	.display_modes = nv3051d_rgxx3_modes,
>   	.num_modes = ARRAY_SIZE(nv3051d_rgxx3_modes),
>   	.width_mm = 70,
>   	.height_mm = 57,
>   	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET |
> +		      MIPI_DSI_CLOCK_NON_CONTINUOUS,
> +};
> +
> +static const struct nv3051d_panel_info nv3051d_rg353p_info = {
> +	.display_modes = nv3051d_rgxx3_modes,
> +	.num_modes = ARRAY_SIZE(nv3051d_rgxx3_modes),
> +	.width_mm = 70,
> +	.height_mm = 57,

Will all the panels for this driver be 70x57? If so, would it be better 
to set display_info.[width_mm|height_mm] directly?

> +	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET,
> +};
> +
> +static const struct nv3051d_panel_info nv3051d_rk2023_info = {
> +	.display_modes = nv3051d_rk2023_modes,
> +	.num_modes = ARRAY_SIZE(nv3051d_rk2023_modes),
> +	.width_mm = 70,
> +	.height_mm = 57,
> +	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET,
>   };
>   
>   static const struct of_device_id newvision_nv3051d_of_match[] = {
> -	{ .compatible = "newvision,nv3051d", .data = &nv3051d_rgxx3_info },
> +	{ .compatible = "anbernic,rg351v-panel", .data = &nv3051d_rg351v_info },
> +	{ .compatible = "anbernic,rg353p-panel", .data = &nv3051d_rg353p_info },
> +	{ .compatible = "powkiddy,rk2023-panel", .data = &nv3051d_rk2023_info },
>   	{ /* sentinel */ }
>   };
> +

I think you can drop this stray newline.

Thanks,

Jessica Zhang

>   MODULE_DEVICE_TABLE(of, newvision_nv3051d_of_match);
>   
>   static struct mipi_dsi_driver newvision_nv3051d_driver = {
> -- 
> 2.34.1
> 

Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

[Heiko Stübner]

Hey Chris,

Am Donnerstag, 19. Oktober 2023, 16:43:56 CEST schrieb Chris Morgan:
> On Thu, Oct 19, 2023 at 11:21:47AM +0200, Krzysztof Kozlowski wrote:
> > On 18/10/2023 18:18, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > > powered by the Rockchip RK3566 SoC.
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > index a349bf4da6bc..a6612185a7ff 100644
> > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > @@ -674,6 +674,11 @@ properties:
> > >            - const: powkiddy,rgb30
> > >            - const: rockchip,rk3566
> > >  
> > > +      - description: Powkiddy RK2023
> > > +        items:
> > > +          - const: powkiddy,rk2023
> > 
> > This cuold be just enum in previous entry :/ but I remember we talked
> > about this once with Heiko.
> 
> For hardware that requires a different device tree, is that possible?
> While most of the devices I've worked on for the RK3566 series are very
> similar for the moment only 1 is identical (the RG353P and the RG353M)
> and can use the same device tree.

In my reply I pointed to the Rock PI 4A/4A+/B/B+/C family, which also has
different devicetrees but is part of the same family of device designs.

So similar Powkiddy RK3568 based gaming handhelds also sound like
a nice family name in the description ;-) .


Heiko

Re: [PATCH 2/5] drm/panel: nv3051d: Add Powkiddy RK2023 Panel Support

[Chris Morgan]

On Thu, Oct 19, 2023 at 10:22:24AM -0700, Jessica Zhang wrote:
> 
> 
> On 10/18/2023 9:18 AM, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Refactor the driver to add support for the powkiddy,rk2023-panel
> > panel. This panel is extremely similar to the rg353p-panel but
> > requires a smaller vertical back porch and isn't as tolerant of
> > higher speeds.
> > 
> > Tested on my RG351V, RG353P, RG353V, and RK2023.
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> 
> Hi Chris,
> 
> Thanks for the patch. Just have a minor question below.
> 
> > ---
> >   .../gpu/drm/panel/panel-newvision-nv3051d.c   | 56 +++++++++++++++----
> >   1 file changed, 45 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> > index 79de6c886292..d24c51503d68 100644
> > --- a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> > +++ b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
> > @@ -28,6 +28,7 @@ struct nv3051d_panel_info {
> >   	unsigned int num_modes;
> >   	u16 width_mm, height_mm;
> >   	u32 bus_flags;
> > +	u32 mode_flags;
> >   };
> >   struct panel_nv3051d {
> > @@ -385,15 +386,7 @@ static int panel_nv3051d_probe(struct mipi_dsi_device *dsi)
> >   	dsi->lanes = 4;
> >   	dsi->format = MIPI_DSI_FMT_RGB888;
> > -	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> > -			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
> > -
> > -	/*
> > -	 * The panel in the RG351V is identical to the 353P, except it
> > -	 * requires MIPI_DSI_CLOCK_NON_CONTINUOUS to operate correctly.
> > -	 */
> > -	if (of_device_is_compatible(dev->of_node, "anbernic,rg351v-panel"))
> > -		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
> > +	dsi->mode_flags = ctx->panel_info->mode_flags;
> >   	drm_panel_init(&ctx->panel, &dsi->dev, &panel_nv3051d_funcs,
> >   		       DRM_MODE_CONNECTOR_DSI);
> > @@ -481,18 +474,59 @@ static const struct drm_display_mode nv3051d_rgxx3_modes[] = {
> >   	},
> >   };
> > -static const struct nv3051d_panel_info nv3051d_rgxx3_info = {
> > +static const struct drm_display_mode nv3051d_rk2023_modes[] = {
> > +	{
> > +		.hdisplay       = 640,
> > +		.hsync_start    = 640 + 40,
> > +		.hsync_end      = 640 + 40 + 2,
> > +		.htotal         = 640 + 40 + 2 + 80,
> > +		.vdisplay       = 480,
> > +		.vsync_start    = 480 + 18,
> > +		.vsync_end      = 480 + 18 + 2,
> > +		.vtotal         = 480 + 18 + 2 + 4,
> > +		.clock          = 24150,
> > +		.flags          = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> > +	},
> > +};
> > +
> > +static const struct nv3051d_panel_info nv3051d_rg351v_info = {
> >   	.display_modes = nv3051d_rgxx3_modes,
> >   	.num_modes = ARRAY_SIZE(nv3051d_rgxx3_modes),
> >   	.width_mm = 70,
> >   	.height_mm = 57,
> >   	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> > +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> > +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET |
> > +		      MIPI_DSI_CLOCK_NON_CONTINUOUS,
> > +};
> > +
> > +static const struct nv3051d_panel_info nv3051d_rg353p_info = {
> > +	.display_modes = nv3051d_rgxx3_modes,
> > +	.num_modes = ARRAY_SIZE(nv3051d_rgxx3_modes),
> > +	.width_mm = 70,
> > +	.height_mm = 57,
> 
> Will all the panels for this driver be 70x57? If so, would it be better to
> set display_info.[width_mm|height_mm] directly?

They are all so far the same size, but I can't guarantee that going forward.
To my knowledge this is the last of the nv3051d devices I'll be working on
in the foreseeable future though, and so far they're all identical in size.

> 
> > +	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> > +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> > +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET,
> > +};
> > +
> > +static const struct nv3051d_panel_info nv3051d_rk2023_info = {
> > +	.display_modes = nv3051d_rk2023_modes,
> > +	.num_modes = ARRAY_SIZE(nv3051d_rk2023_modes),
> > +	.width_mm = 70,
> > +	.height_mm = 57,
> > +	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
> > +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
> > +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET,
> >   };
> >   static const struct of_device_id newvision_nv3051d_of_match[] = {
> > -	{ .compatible = "newvision,nv3051d", .data = &nv3051d_rgxx3_info },
> > +	{ .compatible = "anbernic,rg351v-panel", .data = &nv3051d_rg351v_info },
> > +	{ .compatible = "anbernic,rg353p-panel", .data = &nv3051d_rg353p_info },
> > +	{ .compatible = "powkiddy,rk2023-panel", .data = &nv3051d_rk2023_info },
> >   	{ /* sentinel */ }
> >   };
> > +

Sorry, will fix that in a V2. Thank you.

> 
> I think you can drop this stray newline.
> 
> Thanks,
> 
> Jessica Zhang
> 
> >   MODULE_DEVICE_TABLE(of, newvision_nv3051d_of_match);
> >   static struct mipi_dsi_driver newvision_nv3051d_driver = {
> > -- 
> > 2.34.1
> > 

Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

[Chris Morgan]

On Thu, Oct 19, 2023 at 07:45:17PM +0200, Heiko Stübner wrote:
> Hey Chris,
> 
> Am Donnerstag, 19. Oktober 2023, 16:43:56 CEST schrieb Chris Morgan:
> > On Thu, Oct 19, 2023 at 11:21:47AM +0200, Krzysztof Kozlowski wrote:
> > > On 18/10/2023 18:18, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > > > powered by the Rockchip RK3566 SoC.
> > > > 
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > index a349bf4da6bc..a6612185a7ff 100644
> > > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > @@ -674,6 +674,11 @@ properties:
> > > >            - const: powkiddy,rgb30
> > > >            - const: rockchip,rk3566
> > > >  
> > > > +      - description: Powkiddy RK2023
> > > > +        items:
> > > > +          - const: powkiddy,rk2023
> > > 
> > > This cuold be just enum in previous entry :/ but I remember we talked
> > > about this once with Heiko.
> > 
> > For hardware that requires a different device tree, is that possible?
> > While most of the devices I've worked on for the RK3566 series are very
> > similar for the moment only 1 is identical (the RG353P and the RG353M)
> > and can use the same device tree.
> 
> In my reply I pointed to the Rock PI 4A/4A+/B/B+/C family, which also has
> different devicetrees but is part of the same family of device designs.
> 
> So similar Powkiddy RK3568 based gaming handhelds also sound like
> a nice family name in the description ;-) .

Gotcha, I can do that. Would you like for me to go back and do the same
for the Anbernic devices as well? I can do it as part of a seperate
patch series.

Thank you.

> 
> 
> Heiko
> 
> 

Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

[Heiko Stübner]

Hi Chris,

Am Freitag, 20. Oktober 2023, 17:03:08 CEST schrieb Chris Morgan:
> On Thu, Oct 19, 2023 at 07:45:17PM +0200, Heiko Stübner wrote:
> > Hey Chris,
> > 
> > Am Donnerstag, 19. Oktober 2023, 16:43:56 CEST schrieb Chris Morgan:
> > > On Thu, Oct 19, 2023 at 11:21:47AM +0200, Krzysztof Kozlowski wrote:
> > > > On 18/10/2023 18:18, Chris Morgan wrote:
> > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > > 
> > > > > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > > > > powered by the Rockchip RK3566 SoC.
> > > > > 
> > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
> > > > >  1 file changed, 5 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > index a349bf4da6bc..a6612185a7ff 100644
> > > > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > @@ -674,6 +674,11 @@ properties:
> > > > >            - const: powkiddy,rgb30
> > > > >            - const: rockchip,rk3566
> > > > >  
> > > > > +      - description: Powkiddy RK2023
> > > > > +        items:
> > > > > +          - const: powkiddy,rk2023
> > > > 
> > > > This cuold be just enum in previous entry :/ but I remember we talked
> > > > about this once with Heiko.
> > > 
> > > For hardware that requires a different device tree, is that possible?
> > > While most of the devices I've worked on for the RK3566 series are very
> > > similar for the moment only 1 is identical (the RG353P and the RG353M)
> > > and can use the same device tree.
> > 
> > In my reply I pointed to the Rock PI 4A/4A+/B/B+/C family, which also has
> > different devicetrees but is part of the same family of device designs.
> > 
> > So similar Powkiddy RK3568 based gaming handhelds also sound like
> > a nice family name in the description ;-) .
> 
> Gotcha, I can do that. Would you like for me to go back and do the same
> for the Anbernic devices as well? I can do it as part of a seperate
> patch series.

that doing that for the Anberic devices would be really nice too, so
yes please :-) .

Thanks
Heiko

Re: [PATCH 1/5] dt-bindings: display: panel: Update NewVision NV3051D compatibles

[Rob Herring]

On Thu, Oct 19, 2023 at 09:50:38AM -0500, Chris Morgan wrote:
> On Thu, Oct 19, 2023 at 11:22:19AM +0200, Krzysztof Kozlowski wrote:
> > On 18/10/2023 18:18, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Update the NewVision NV3051D compatible strings by adding a new panel,
> > > the powkiddy,rk2023-panel, and removing another entry, the
> > > anbernic,rg353v-panel. The rg353v-panel is exactly identical to the
> > > rg353p-panel and is not currently in use by any existing device tree.
> > > The rk2023-panel is similar to the rg353p-panel but has slightly
> > > different timings.
> > 
> > This still does not explain me why do you want to remove old panel.
> 
> When I originally wrote the driver I only had one piece of hardware
> and I set the compatible string in the driver as newvision,nv3051d.
> Unfortunately since then I've found 2 more devices in use that are
> *just* different enough to require the driver to do things a bit
> differently. In the case of the anbernic,rg351v-panel I need to
> enable a new DSI flag; in the case of the powkiddy,rk2023-panel I need
> to decrease the vertical back porch and drop the higher frequency
> timings.
> 
> The best way to accomplish this was to change the strategy from having
> a single binding in the driver of newvision,nv3051d to a binding for
> each distinct hardware where the differences apply. 

Exactly why the DT maintainers annoyingly ask for specific compatible 
strings which may not be used immediately.

> Note that I've
> looked at querying the DSI panel ID, but for each device the value
> is identical (so it can't be used to differentiate the hardware sadly).
> So the driver now has 3 different compatible strings. I could in this
> case add a 4th compatible string of anbernic,rg353v-panel but it would
> be identical to anbernic,rg353p-panel. For the moment we are using
> anbernic,rg353p-panel everywhere (including the rg353v), so it makes
> sense to drop this unused value while we can, at least to me.

Your reasoning is the compatible string is unused, so remove it. 

If there's some reasoning about how the 2 panels are the same hardware 
or the rg353v is never going to be used or show up at some point, then 
that would be a reason to remove.

You could also say the rg353v is just wrong because it should have a 
fallback compatible to rg353p and rather than fix it, just remove it 
for now since there are no known users of it.

Rob

Re: [PATCH 1/5] dt-bindings: display: panel: Update NewVision NV3051D compatibles

[Chris Morgan]

On Tue, Oct 24, 2023 at 01:27:55PM -0500, Rob Herring wrote:
> On Thu, Oct 19, 2023 at 09:50:38AM -0500, Chris Morgan wrote:
> > On Thu, Oct 19, 2023 at 11:22:19AM +0200, Krzysztof Kozlowski wrote:
> > > On 18/10/2023 18:18, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > Update the NewVision NV3051D compatible strings by adding a new panel,
> > > > the powkiddy,rk2023-panel, and removing another entry, the
> > > > anbernic,rg353v-panel. The rg353v-panel is exactly identical to the
> > > > rg353p-panel and is not currently in use by any existing device tree.
> > > > The rk2023-panel is similar to the rg353p-panel but has slightly
> > > > different timings.
> > > 
> > > This still does not explain me why do you want to remove old panel.
> > 
> > When I originally wrote the driver I only had one piece of hardware
> > and I set the compatible string in the driver as newvision,nv3051d.
> > Unfortunately since then I've found 2 more devices in use that are
> > *just* different enough to require the driver to do things a bit
> > differently. In the case of the anbernic,rg351v-panel I need to
> > enable a new DSI flag; in the case of the powkiddy,rk2023-panel I need
> > to decrease the vertical back porch and drop the higher frequency
> > timings.
> > 
> > The best way to accomplish this was to change the strategy from having
> > a single binding in the driver of newvision,nv3051d to a binding for
> > each distinct hardware where the differences apply. 
> 
> Exactly why the DT maintainers annoyingly ask for specific compatible 
> strings which may not be used immediately.

You're not wrong. Sorry for making this difficult. I should have done
it this way from the start.

> 
> > Note that I've
> > looked at querying the DSI panel ID, but for each device the value
> > is identical (so it can't be used to differentiate the hardware sadly).
> > So the driver now has 3 different compatible strings. I could in this
> > case add a 4th compatible string of anbernic,rg353v-panel but it would
> > be identical to anbernic,rg353p-panel. For the moment we are using
> > anbernic,rg353p-panel everywhere (including the rg353v), so it makes
> > sense to drop this unused value while we can, at least to me.
> 
> Your reasoning is the compatible string is unused, so remove it. 
> 
> If there's some reasoning about how the 2 panels are the same hardware 
> or the rg353v is never going to be used or show up at some point, then 
> that would be a reason to remove.

The compatible string of 353v-panel is unused, and the hardware is
identical to the 353p-panel (so only one string is necessary). Sorry
if that wasn't clear.

Panel 1 - The original anbernic,rg353p-panel which is also
anbernic,rg353v-panel.

Panel 2 - anbernic,rg351v-panel. This is almost identical to Panel 1
except it requires an additional flag.

Panel 3 - powkiddy,rk2023-panel. This is almost identical to Panel 1
except it requires a change to the VBP timing parameter and isn't
tolerant of speeds much higher than 60hz.

The issue I had is I originally wrote the driver checking for the
newvision,nv3051d compatible string which worked fine when there was
only 1 panel type. When I added support for the 351v-panel I *should*
have changed how the compatible string was handled, but instead I
simply added a check in the probe function to look for the secondary
string of "anbernic,rg351v-panel". When the 3rd panel type of
"powkiddy,rk2023-panel" was needed I took this time to correct the
driver and do it the right way by checking for the specific
compatibles.

Thank you, and sorry for the headaches this caused you.
Chris

> 
> You could also say the rg353v is just wrong because it should have a 
> fallback compatible to rg353p and rather than fix it, just remove it 
> for now since there are no known users of it.
> 
> Rob

Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

[Chris Morgan]

On Tue, Oct 24, 2023 at 05:47:37PM +0200, Heiko Stübner wrote:
> Hi Chris,
> 
> Am Freitag, 20. Oktober 2023, 17:03:08 CEST schrieb Chris Morgan:
> > On Thu, Oct 19, 2023 at 07:45:17PM +0200, Heiko Stübner wrote:
> > > Hey Chris,
> > > 
> > > Am Donnerstag, 19. Oktober 2023, 16:43:56 CEST schrieb Chris Morgan:
> > > > On Thu, Oct 19, 2023 at 11:21:47AM +0200, Krzysztof Kozlowski wrote:
> > > > > On 18/10/2023 18:18, Chris Morgan wrote:
> > > > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > > > 
> > > > > > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > > > > > powered by the Rockchip RK3566 SoC.
> > > > > > 
> > > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +++++
> > > > > >  1 file changed, 5 insertions(+)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > > index a349bf4da6bc..a6612185a7ff 100644
> > > > > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > > @@ -674,6 +674,11 @@ properties:
> > > > > >            - const: powkiddy,rgb30
> > > > > >            - const: rockchip,rk3566
> > > > > >  
> > > > > > +      - description: Powkiddy RK2023
> > > > > > +        items:
> > > > > > +          - const: powkiddy,rk2023
> > > > > 
> > > > > This cuold be just enum in previous entry :/ but I remember we talked
> > > > > about this once with Heiko.
> > > > 
> > > > For hardware that requires a different device tree, is that possible?
> > > > While most of the devices I've worked on for the RK3566 series are very
> > > > similar for the moment only 1 is identical (the RG353P and the RG353M)
> > > > and can use the same device tree.
> > > 
> > > In my reply I pointed to the Rock PI 4A/4A+/B/B+/C family, which also has
> > > different devicetrees but is part of the same family of device designs.
> > > 
> > > So similar Powkiddy RK3568 based gaming handhelds also sound like
> > > a nice family name in the description ;-) .
> > 
> > Gotcha, I can do that. Would you like for me to go back and do the same
> > for the Anbernic devices as well? I can do it as part of a seperate
> > patch series.
> 
> that doing that for the Anberic devices would be really nice too, so
> yes please :-) .
> 
> Thanks
> Heiko
> 
> 

Did you want me to resubmit the series or just push the changes on top
of this one? I'll get them both queued up.

Thank you.

Re: [PATCH 2/5] drm/panel: nv3051d: Add Powkiddy RK2023 Panel Support

[Jessica Zhang]

On 10/20/2023 8:02 AM, Chris Morgan wrote:
> On Thu, Oct 19, 2023 at 10:22:24AM -0700, Jessica Zhang wrote:
>>
>>
>> On 10/18/2023 9:18 AM, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> Refactor the driver to add support for the powkiddy,rk2023-panel
>>> panel. This panel is extremely similar to the rg353p-panel but
>>> requires a smaller vertical back porch and isn't as tolerant of
>>> higher speeds.
>>>
>>> Tested on my RG351V, RG353P, RG353V, and RK2023.
>>>
>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>
>> Hi Chris,
>>
>> Thanks for the patch. Just have a minor question below.
>>
>>> ---
>>>    .../gpu/drm/panel/panel-newvision-nv3051d.c   | 56 +++++++++++++++----
>>>    1 file changed, 45 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
>>> index 79de6c886292..d24c51503d68 100644
>>> --- a/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
>>> +++ b/drivers/gpu/drm/panel/panel-newvision-nv3051d.c
>>> @@ -28,6 +28,7 @@ struct nv3051d_panel_info {
>>>    	unsigned int num_modes;
>>>    	u16 width_mm, height_mm;
>>>    	u32 bus_flags;
>>> +	u32 mode_flags;
>>>    };
>>>    struct panel_nv3051d {
>>> @@ -385,15 +386,7 @@ static int panel_nv3051d_probe(struct mipi_dsi_device *dsi)
>>>    	dsi->lanes = 4;
>>>    	dsi->format = MIPI_DSI_FMT_RGB888;
>>> -	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
>>> -			  MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
>>> -
>>> -	/*
>>> -	 * The panel in the RG351V is identical to the 353P, except it
>>> -	 * requires MIPI_DSI_CLOCK_NON_CONTINUOUS to operate correctly.
>>> -	 */
>>> -	if (of_device_is_compatible(dev->of_node, "anbernic,rg351v-panel"))
>>> -		dsi->mode_flags |= MIPI_DSI_CLOCK_NON_CONTINUOUS;
>>> +	dsi->mode_flags = ctx->panel_info->mode_flags;
>>>    	drm_panel_init(&ctx->panel, &dsi->dev, &panel_nv3051d_funcs,
>>>    		       DRM_MODE_CONNECTOR_DSI);
>>> @@ -481,18 +474,59 @@ static const struct drm_display_mode nv3051d_rgxx3_modes[] = {
>>>    	},
>>>    };
>>> -static const struct nv3051d_panel_info nv3051d_rgxx3_info = {
>>> +static const struct drm_display_mode nv3051d_rk2023_modes[] = {
>>> +	{
>>> +		.hdisplay       = 640,
>>> +		.hsync_start    = 640 + 40,
>>> +		.hsync_end      = 640 + 40 + 2,
>>> +		.htotal         = 640 + 40 + 2 + 80,
>>> +		.vdisplay       = 480,
>>> +		.vsync_start    = 480 + 18,
>>> +		.vsync_end      = 480 + 18 + 2,
>>> +		.vtotal         = 480 + 18 + 2 + 4,
>>> +		.clock          = 24150,
>>> +		.flags          = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
>>> +	},
>>> +};
>>> +
>>> +static const struct nv3051d_panel_info nv3051d_rg351v_info = {
>>>    	.display_modes = nv3051d_rgxx3_modes,
>>>    	.num_modes = ARRAY_SIZE(nv3051d_rgxx3_modes),
>>>    	.width_mm = 70,
>>>    	.height_mm = 57,
>>>    	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>>> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
>>> +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET |
>>> +		      MIPI_DSI_CLOCK_NON_CONTINUOUS,
>>> +};
>>> +
>>> +static const struct nv3051d_panel_info nv3051d_rg353p_info = {
>>> +	.display_modes = nv3051d_rgxx3_modes,
>>> +	.num_modes = ARRAY_SIZE(nv3051d_rgxx3_modes),
>>> +	.width_mm = 70,
>>> +	.height_mm = 57,
>>
>> Will all the panels for this driver be 70x57? If so, would it be better to
>> set display_info.[width_mm|height_mm] directly?
> 
> They are all so far the same size, but I can't guarantee that going forward.
> To my knowledge this is the last of the nv3051d devices I'll be working on
> in the foreseeable future though, and so far they're all identical in size.

Got it, if it's not guaranteed might be better to leave it as it then. 
Thanks for clarifying.

BR,

Jessica Zhang

> 
>>
>>> +	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>>> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
>>> +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET,
>>> +};
>>> +
>>> +static const struct nv3051d_panel_info nv3051d_rk2023_info = {
>>> +	.display_modes = nv3051d_rk2023_modes,
>>> +	.num_modes = ARRAY_SIZE(nv3051d_rk2023_modes),
>>> +	.width_mm = 70,
>>> +	.height_mm = 57,
>>> +	.bus_flags = DRM_BUS_FLAG_DE_LOW | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
>>> +	.mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
>>> +		      MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET,
>>>    };
>>>    static const struct of_device_id newvision_nv3051d_of_match[] = {
>>> -	{ .compatible = "newvision,nv3051d", .data = &nv3051d_rgxx3_info },
>>> +	{ .compatible = "anbernic,rg351v-panel", .data = &nv3051d_rg351v_info },
>>> +	{ .compatible = "anbernic,rg353p-panel", .data = &nv3051d_rg353p_info },
>>> +	{ .compatible = "powkiddy,rk2023-panel", .data = &nv3051d_rk2023_info },
>>>    	{ /* sentinel */ }
>>>    };
>>> +
> 
> Sorry, will fix that in a V2. Thank you.
> 
>>
>> I think you can drop this stray newline.
>>
>> Thanks,
>>
>> Jessica Zhang
>>
>>>    MODULE_DEVICE_TABLE(of, newvision_nv3051d_of_match);
>>>    static struct mipi_dsi_driver newvision_nv3051d_driver = {
>>> -- 
>>> 2.34.1
>>>