On Wed, Jan 09, 2019 at 02:53:31PM +0100, Linus Walleij wrote:
[...]
> diff --git a/drivers/gpu/drm/panel/panel-tpo-tpg110.c
> b/drivers/gpu/drm/panel/panel-tpo-tpg110.c
[...]
> +static int tpg110_startup(struct tpg110 *tpg)
> +{
> + u8 val;
> + int i;
> +
> + /* De-assert the reset signal */
> + gpiod_set_value_cansleep(tpg->grestb, 0);
> + mdelay(1);
Does this have to be the spinning variant? This seems to be only used in
the probe path, so doesn't have to be atomic.
> + dev_info(tpg->dev, "de-asserted GRESTB\n");
Maybe turn this into dev_dbg()?
> +
> + /* Test display communication */
> + tpg110_write_reg(tpg, TPG110_TEST, 0x55);
> + val = tpg110_read_reg(tpg, TPG110_TEST);
> + if (val != 0x55) {
> + dev_err(tpg->dev, "failed communication test\n");
> + return -ENODEV;
> + }
> +
> + val = tpg110_read_reg(tpg, TPG110_CHIPID);
> + dev_info(tpg->dev, "TPG110 chip ID: %d version: %d\n",
> + val >> 4, val & 0x0f);
> +
> + /* Show display resolution */
> + val = tpg110_read_reg(tpg, TPG110_CTRL1);
> + val &= TPG110_RES_MASK;
> + switch (val) {
> + case TPG110_RES_400X240_D:
> + dev_info(tpg->dev,
> + "IN 400x240 RGB -> OUT 800x480 RGB (dual scan)");
> + break;
> + case TPG110_RES_480X272_D:
> + dev_info(tpg->dev,
> + "IN 480x272 RGB -> OUT 800x480 RGB (dual scan)");
> + break;
> + case TPG110_RES_480X640:
> + dev_info(tpg->dev, "480x640 RGB");
> + break;
> + case TPG110_RES_480X272:
> + dev_info(tpg->dev, "480x272 RGB");
> + break;
> + case TPG110_RES_640X480:
> + dev_info(tpg->dev, "640x480 RGB");
> + break;
> + case TPG110_RES_800X480:
> + dev_info(tpg->dev, "800x480 RGB");
> + break;
> + default:
> + dev_info(tpg->dev, "ILLEGAL RESOLUTION");
dev_err()? Also, perhaps make this some proper error message and include
the invalid value?
Also I just noticed that the above informational messages all lack a \n
at the end.
The above is also very verbose, do we really need all that information
in the kernel log?
> + break;
> + }
> +
> + /* From the producer side, this is the same resolution */
> + if (val == TPG110_RES_480X272_D)
> + val = TPG110_RES_480X272;
> +
> + for (i = 0; i < ARRAY_SIZE(tpg110_modes); i++) {
> + const struct tpg110_panel_mode *pm;
> +
> + pm = &tpg110_modes[i];
> + if (pm->magic == val) {
> + tpg->panel_mode = pm;
> + break;
> + }
> + }
> + if (i == ARRAY_SIZE(tpg110_modes)) {
> + dev_err(tpg->dev, "unsupported mode (%02x) detected\n",
> + val);
> + return -ENODEV;
> + }
> +
> + val = tpg110_read_reg(tpg, TPG110_CTRL2);
> + dev_info(tpg->dev, "resolution and standby is controlled by %s\n",
> + (val & TPG110_CTRL2_RES_PM_CTRL) ? "software" : "hardware");
> + /* Take control over resolution and standby */
> + val |= TPG110_CTRL2_RES_PM_CTRL;
> + tpg110_write_reg(tpg, TPG110_CTRL2, val);
> +
> + return 0;
> +}
> +
> +static int tpg110_disable(struct drm_panel *panel)
> +{
> + struct tpg110 *tpg = to_tpg110(panel);
> + u8 val;
> +
> + /* Put chip into standby */
> + val = tpg110_read_reg(tpg, TPG110_CTRL2_PM);
> + val &= ~TPG110_CTRL2_PM;
> + tpg110_write_reg(tpg, TPG110_CTRL2_PM, val);
> +
> + if (tpg->backlight) {
> + tpg->backlight->props.power = FB_BLANK_POWERDOWN;
> + tpg->backlight->props.state |= BL_CORE_FBBLANK;
> + backlight_update_status(tpg->backlight);
> + }
backlight_disable()?
> +
> + return 0;
> +}
> +
> +static int tpg110_enable(struct drm_panel *panel)
> +{
> + struct tpg110 *tpg = to_tpg110(panel);
> + u8 val;
> +
> + if (tpg->backlight) {
> + tpg->backlight->props.state &= ~BL_CORE_FBBLANK;
> + tpg->backlight->props.power = FB_BLANK_UNBLANK;
> + backlight_update_status(tpg->backlight);
> + }
backlight_enable()?
Thierry
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
