Hi Jitao.
On Sat, Oct 12, 2019 at 11:07:14AM +0800, Jitao Shi wrote:
> Add driver for BOE tv101wum-nl6 panel is a 10.1" 1200x1920 panel.
>
> Signed-off-by: Jitao Shi <jitao.shi@xxxxxxxxxxxx>
> Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> ---
> +
> +struct boe_panel {
> + struct drm_panel base;
> + struct mipi_dsi_device *dsi;
> +
> + const struct panel_desc *desc;
> +
> + struct backlight_device *backlight;
drm_panel has gained backlight support in drm-misc-next.
Please use this.
> + struct regulator *pp1800;
> + struct regulator *avee;
> + struct regulator *avdd;
> + struct gpio_desc *enable_gpio;
> +
> + bool prepared;
> + bool enabled;
This flag can be dropped when using drm_panel backlight support.
> +
> + const struct drm_display_mode *mode;
> +};
> +
> +static int boe_panel_disable(struct drm_panel *panel)
> +{
> + struct boe_panel *boe = to_boe_panel(panel);
> +
> + if (!boe->enabled)
> + return 0;
> +
> + backlight_disable(boe->backlight);
> +
> + boe->enabled = false;
> +
> + return 0;
> +}
This function can be dropped when using the drm_panel backlight support.
> +
> +static int boe_panel_enable(struct drm_panel *panel)
> +{
> + struct boe_panel *boe = to_boe_panel(panel);
> + int ret;
> +
> + if (boe->enabled)
> + return 0;
> +
> + ret = backlight_enable(boe->backlight);
> + if (ret) {
> + dev_err(panel->dev, "Failed to enable backlight %d\n",
> + ret);
> + return ret;
> + }
> +
> + boe->enabled = true;
> +
> + return 0;
> +}
This function can be dropped when using the drm_panel backlight support.
> +
> +static const struct drm_display_mode boe_tv101wum_nl6_default_mode = {
> + .clock = 159425,
> + .hdisplay = 1200,
> + .hsync_start = 1200 + 100,
> + .hsync_end = 1200 + 100 + 40,
> + .htotal = 1200 + 100 + 40 + 24,
> + .vdisplay = 1920,
> + .vsync_start = 1920 + 10,
> + .vsync_end = 1920 + 10 + 14,
> + .vtotal = 1920 + 10 + 14 + 4,
> + .vrefresh = 60,
> +};
> +
> +static const struct panel_desc boe_tv101wum_nl6_desc = {
> + .modes = &boe_tv101wum_nl6_default_mode,
> + .bpc = 8,
> + .size = {
> + .width_mm = 135,
> + .height_mm = 216,
> + },
> + .lanes = 4,
> + .format = MIPI_DSI_FMT_RGB888,
> + .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_SYNC_PULSE |
> + MIPI_DSI_MODE_LPM,
> + .init_cmds = boe_init_cmd,
> +};
> +
> +static int boe_panel_get_modes(struct drm_panel *panel)
This function now takes drm_connector as second argument.
See drm-misc-next.
> +{
> + struct boe_panel *boe = to_boe_panel(panel);
> + const struct drm_display_mode *m = boe->desc->modes;
> + struct drm_display_mode *mode;
> +
> + mode = drm_mode_duplicate(panel->drm, m);
Here you will need to use connector->dev to get the drm_device.
> + if (!mode) {
> + dev_err(panel->dev, "failed to add mode %ux%u@%u\n",
> + m->hdisplay, m->vdisplay, m->vrefresh);
> + return -ENOMEM;
> + }
> +
> + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> + drm_mode_set_name(mode);
> + drm_mode_probed_add(panel->connector, mode);
> +
> + panel->connector->display_info.width_mm = boe->desc->size.width_mm;
> + panel->connector->display_info.height_mm = boe->desc->size.height_mm;
> + panel->connector->display_info.bpc = boe->desc->bpc;
There is no connector in drm_panel today.
Use the connector in supplied as second argument.
> +
> + return 1;
> +}
> +
> + gpiod_set_value(boe->enable_gpio, 0);
> +
> + boe->backlight = devm_of_find_backlight(dev);
> + if (IS_ERR(boe->backlight))
> + return PTR_ERR(boe->backlight);
Move this after drm_panel_init and use drm_panel_of_backlight()
The binding say that backlight is mandatory. This is really not checked
here. And I *think* backlight is optional.
> +
> + drm_panel_init(&boe->base);
Here you need to specify connector_type as second argument.
> +
> + boe->base.funcs = &boe_panel_funcs;
> + boe->base.dev = &boe->dsi->dev;
> +
> + return drm_panel_add(&boe->base);
> +}
> +
> +}
> +
> +static int boe_panel_remove(struct mipi_dsi_device *dsi)
> +{
> + struct boe_panel *boe = mipi_dsi_get_drvdata(dsi);
> + int ret;
> +
> + ret = boe_panel_disable(&boe->base);
> + if (ret < 0)
> + dev_err(&dsi->dev, "failed to disable panel: %d\n", ret);
> +
> + ret = mipi_dsi_detach(dsi);
> + if (ret < 0)
> + dev_err(&dsi->dev, "failed to detach from DSI host: %d\n", ret);
> +
> + if (boe->base.dev)
> + drm_panel_remove(&boe->base);
> +
> + return 0;
> +}
This should just be:
boe_panel_shutdown();
ret = mipi_dsi_detach();
if (ret < 0)
...
drm_panel_remove();
> +
> +static void boe_panel_shutdown(struct mipi_dsi_device *dsi)
> +{
> + struct boe_panel *boe = mipi_dsi_get_drvdata(dsi);
> +
> + boe_panel_disable(&boe->base);
> +}
Please use drm_panel variants here.
And then I had expected to see:
drm_panel_disable();
drm_panel_unprepare();
> +
> +static const struct of_device_id boe_of_match[] = {
> + { .compatible = "boe,tv101wum-nl6",
> + .data = &boe_tv101wum_nl6_desc
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, boe_of_match);
> +
> +static struct mipi_dsi_driver boe_panel_driver = {
> + .driver = {
> + .name = "panel-boe-tv101wum-nl6",
> + .of_match_table = boe_of_match,
> + },
> + .probe = boe_panel_probe,
> + .remove = boe_panel_remove,
> + .shutdown = boe_panel_shutdown,
> +};
> +module_mipi_dsi_driver(boe_panel_driver);
> +
> +MODULE_AUTHOR("Jitao Shi <jitao.shi@xxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("BOE tv101wum-nl6 1200x1920 video mode panel driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.21.0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
