On 10/18/2023 9:18 AM, Chris Morgan wrote:
From: Chris Morgan <macromorgan@xxxxxxxxxxx>
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@xxxxxxxxxxx>
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
