mbox series

[V2,0/4] rockchip: Add Powkiddy RK2023

Message ID 20231109215007.66826-1-macroalpha82@gmail.com
Headers show
Series rockchip: Add Powkiddy RK2023 | expand

Message

Chris Morgan Nov. 9, 2023, 9:50 p.m. UTC 
From: Chris Morgan <macromorgan@hotmail.com>

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

Changes since V1:
 - Necessary clock changes have been accepted to mainline, so removed
   from this series.
   https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?id=f1db0865b4628d5e2e85347350c077a71f0629d2
 - Combined Powkiddy RK3566 devices in devicetree documentation.
   Dropped ack from binding as this change is vastly different than
   the previous update.
 - Updated panel driver to hold panel in reset status after unprepare.

Chris Morgan (4):
  dt-bindings: display: panel: Update NewVision NV3051D compatibles
  nv3051d: Add Powkiddy RK2023 Panel Support
  dt-bindings: arm: rockchip: Add Powkiddy RK2023
  arm64: dts: rockchip: add Powkiddy RK2023

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

Comments

Krzysztof Kozlowski Nov. 10, 2023, 1:14 p.m. UTC | #1 
On 09/11/2023 22:50, Chris Morgan wrote:
> 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 a18f33bf0c0e..f969618da352 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;
> +};

If you have to delete so many nodes and properties, this means your
common DTSI is not that common.

Common DTSI means there is shared hardware. If you take shared hardware
and put it into another board, pieces of the shared hardware do not
disappear.


Best regards,
Krzysztof
Chris Morgan Nov. 10, 2023, 2:30 p.m. UTC | #2 
On Fri, Nov 10, 2023 at 02:14:23PM +0100, Krzysztof Kozlowski wrote:
> On 09/11/2023 22:50, Chris Morgan wrote:
> > 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 a18f33bf0c0e..f969618da352 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;
> > +};
> 
> If you have to delete so many nodes and properties, this means your
> common DTSI is not that common.
> 
> Common DTSI means there is shared hardware. If you take shared hardware
> and put it into another board, pieces of the shared hardware do not
> disappear.
> 
> 
> Best regards,
> Krzysztof
> 

I can redo this with a different top-level dtsi so I won't have to
delete so much if that helps. I'll redo both the RGB30 and RK2023
so that they use a common dtsi and no longer rely on the Anberic
one. I'll do that and submit a V3.

Thank you.
Krzysztof Kozlowski Nov. 10, 2023, 3:02 p.m. UTC | #3 
On 10/11/2023 15:30, Chris Morgan wrote:
> On Fri, Nov 10, 2023 at 02:14:23PM +0100, Krzysztof Kozlowski wrote:

>>> +
>>> +/delete-node/ &adc_keys;
>>> +
>>> +&chosen {
>>> +	/delete-property/ stdout-path;
>>> +};
>>
>> If you have to delete so many nodes and properties, this means your
>> common DTSI is not that common.
>>
>> Common DTSI means there is shared hardware. If you take shared hardware
>> and put it into another board, pieces of the shared hardware do not
>> disappear.
>>
>>
>> Best regards,
>> Krzysztof
>>
> 
> I can redo this with a different top-level dtsi so I won't have to
> delete so much if that helps. I'll redo both the RGB30 and RK2023
> so that they use a common dtsi and no longer rely on the Anberic
> one. I'll do that and submit a V3.

I am not sure whether you need another top-level DTSI, but maybe rather
move things from common DTSI to each boards. Or you are using wrong
common DTSI. You are supposed to reference common DTSI if the hardware
parts are common. Don't use some DTSI just because DTS looks similar.

I did not investigate your hardware to give recommendation how DTS
should look like. But judging from node removals, something is not done
obvious way.

Best regards,
Krzysztof
Jessica Zhang Nov. 15, 2023, 10:37 p.m. UTC | #4 
On 11/9/2023 1:50 PM, 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. Note that while all of these panels are identical in
> size (70x57) it is possible future panels may not be.
> 
> Tested on my RG351V, RG353P, RG353V, and RK2023.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>   .../gpu/drm/panel/panel-newvision-nv3051d.c   | 57 +++++++++++++++----
>   1 file changed, 46 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..94d89ffd596b 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 {
> @@ -261,6 +262,8 @@ static int panel_nv3051d_unprepare(struct drm_panel *panel)
>   
>   	usleep_range(10000, 15000);
>   
> +	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +

Hi Chris,

Is this to address a suspend/resume issue similar to what was done here [1]?

If so, I think maybe this can go as a separate Fixes patch.

Thanks,

Jessica Zhang

[1] https://patchwork.freedesktop.org/patch/567670/?series=126479&rev=1

>   	regulator_disable(ctx->vdd);
>   
>   	return 0;
> @@ -385,15 +388,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,16 +476,56 @@ 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);
> -- 
> 2.34.1
>