Den 30.06.2021 20.28, skrev Linus Walleij:
> This adds a driver for panels based on the WideChips WS2401 display
> controller. This display controller is used in the Samsung LMS380KF01
> display found in the Samsung GT-I8160 (Codina) mobile phone and
> possibly others.
>
> As is common with Samsung displays manufacturer commands are necessary
> to configure the display to a working state.
>
> The display optionally supports internal backlight control, but can
> also use an external backlight.
>
> This driver re-uses the DBI infrastructure to communicate with the
> display.
>
> Cc: phone-devel@xxxxxxxxxxxxxxx
> Cc: Douglas Anderson <dianders@xxxxxxxxxxxx>
> Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> diff --git a/drivers/gpu/drm/panel/panel-widechips-ws2401.c b/drivers/gpu/drm/panel/panel-widechips-ws2401.c
> +static void ws2401_read_mtp_id(struct ws2401 *ws)
> +{
> + struct mipi_dbi *dbi = &ws->dbi;
> + u8 id1, id2, id3;
> + int ret;
> +
> + ret = mipi_dbi_command_read(dbi, WS2401_READ_ID1, &id1);
> + if (ret) {
> + dev_err(ws->dev, "unable to read MTP ID 1\n");
> + return;
> + }
> + ret = mipi_dbi_command_read(dbi, WS2401_READ_ID2, &id1);
Didn't spot this earlier, but you're reading ID2 and ID3 into id1.
> + if (ret) {
> + dev_err(ws->dev, "unable to read MTP ID 2\n");
> + return;
> + }
> + ret = mipi_dbi_command_read(dbi, WS2401_READ_ID3, &id1);
> + if (ret) {
> + dev_err(ws->dev, "unable to read MTP ID 3\n");
> + return;
> + }
> + dev_info(ws->dev, "MTP ID: %02x %02x %02x\n", id1, id2, id3);
> +}
> +static int ws2401_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct ws2401 *ws;
> + int ret;
> +
> + ws = devm_kzalloc(dev, sizeof(*ws), GFP_KERNEL);
> + if (!ws)
> + return -ENOMEM;
> + ws->dev = dev;
> +
> + /*
> + * VCI is the analog voltage supply
> + * VCCIO is the digital I/O voltage supply
> + */
> + ws->regulators[0].supply = "vci";
> + ws->regulators[1].supply = "vccio";
> + ret = devm_regulator_bulk_get(dev,
> + ARRAY_SIZE(ws->regulators),
> + ws->regulators);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
> + ws->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(ws->reset)) {
> + ret = PTR_ERR(ws->reset);
> + return dev_err_probe(dev, ret, "no RESET GPIO\n");
> + }
> +
> + ret = mipi_dbi_spi_init(spi, &ws->dbi, NULL);
> + if (ret)
> + return dev_err_probe(dev, ret, "MIPI DBI init failed\n");
> + ws->dbi.read_commands = ws2401_dbi_read_commands;
> +
> + ws2401_power_on(ws);
> + ws2401_read_mtp_id(ws);
> + ws2401_power_off(ws);
> +
> + drm_panel_init(&ws->panel, dev, &ws2401_drm_funcs,
> + DRM_MODE_CONNECTOR_DPI);
> +
> + ret = drm_panel_of_backlight(&ws->panel);
I still don't see how internal backlight should work, have you tried it?
Tracking down the call chain, drm_panel_of_backlight() can only return
0, -EINVAL, -ENOMEM and -EPROBE_DEFER. It returns 0 whether the
backlight property exists or not.
I would do something like this:
if (ret)
return dev_err_probe(dev, ret, "failed to get backlight");
if (!ws->panel.backlight) {
> + dev_dbg(dev, "no external backlight, using internal backlight\n");
> + ws->bl = devm_backlight_device_register(dev, "ws2401", dev, ws,
> + &ws2401_bl_ops, &ws2401_bl_props);
> + if (IS_ERR(ws->bl))
> + return dev_err_probe(dev, PTR_ERR(ws->bl),
> + "failed to register backlight device\n");
> + ws->panel.backlight = ws->bl;
> + } else {
> + dev_dbg(dev, "using external backlight\n");
> + }
> +
> + spi_set_drvdata(spi, ws);
> +
> + drm_panel_add(&ws->panel);
> + dev_dbg(dev, "added panel\n");
> +
> + return 0;
> +}
Noralf.
