Provide a small convenience wrapper that set/get the display brightness value
Cc: John Stultz john.stultz@linaro.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Archit Taneja archit.taneja@gmail.com Cc: Rob Clark robdclark@gmail.com Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Thierry Reding thierry.reding@gmail.com Signed-off-by: Vinay Simha BN simhavcs@gmail.com
--- v1: *tested in nexus7 2nd gen.
v2: * implemented jani review comments -functions name mapped accordingly -bl value increased from 0xff to 0xffff -backlight interface will be handled in panel driver, so it is moved from the mipi_dsi helper function --- drivers/gpu/drm/drm_mipi_dsi.c | 49 ++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dsi.h | 4 ++++ 2 files changed, 53 insertions(+)
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index 49311fc..2c03784 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -1041,6 +1041,55 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) } EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
+/** + * mipi_dsi_dcs_get_display_brightness() - gets the current brightness value + * of the display + * @dsi: DSI peripheral device + * @brightness: brightness value + * + * Return: 0 on success or a negative error code on failure. + */ +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, + u16 *brightness) +{ + ssize_t err; + + err = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS, + brightness, sizeof(*brightness)); + if (err < 0) { + if (err == 0) + err = -ENODATA; + + return err; + } + + return 0; +} +EXPORT_SYMBOL(mipi_dsi_dcs_get_display_brightness); + +/** + * mipi_dsi_dcs_set_display_brightness() - sets the brightness value of + * the display + * @dsi: DSI peripheral device + * @brightness: brightness value + * + * Return: 0 on success or a negative error code on failure. + */ +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, + u16 brightness) +{ + ssize_t err; + u8 bl_value[2] = { brightness & 0xff, brightness >> 8 }; + + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS, + bl_value, sizeof(bl_value)); + if (err < 0) + return err; + + return 0; +} +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_brightness); + static int mipi_dsi_drv_probe(struct device *dev) { struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver); diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h index 72f5b15..4d77bb0 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -270,6 +270,10 @@ int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi); int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi, enum mipi_dsi_dcs_tear_mode mode); int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format); +int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi, + u16 *brightness); +int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi, + u16 brightness);
/** * struct mipi_dsi_driver - DSI driver
Add support for the JDI LT070ME05000 WUXGA DSI panel used in Nexus 7 2013 devices.
Programming sequence for the panel is was originally found in the android-msm-flo-3.4-lollipop-release branch from: https://android.googlesource.com/kernel/msm.git
And video mode setting is from dsi-panel-jdi-dualmipi1-video.dtsi file in: git://codeaurora.org/kernel/msm-3.10.git LNX.LA.3.6_rb1.27
Cc: Archit Taneja archit.taneja@gmail.com Cc: Rob Clark robdclark@gmail.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: John Stultz john.stultz@linaro.org Cc: Emil Velikov emil.l.velikov@gmail.com Cc: Thierry Reding thierry.reding@gmail.com Cc: David Airlie airlied@linux.ie Signed-off-by: Sumit Semwal sumit.semwal@linaro.org Signed-off-by: John Stultz john.stultz@linaro.org Signed-off-by: Vinay Simha BN simhavcs@gmail.com
--- v1: * sumit ported to drm/panel framework, john cherry-picked to mainline, folded down other fixes from Vinay and Archit, vinay removed interface setting cmd mode, video mode panel selected v2: * incorporated code reviews from theiry, archit code style, alphabetical soring in Makefile, Kconfig, regulator_bulk, arrays of u8, generic helper function, documentation bindings,
v3: * dcs backlight support added * tested this panel driver in nexus7 2013 device
v4: * backlight interface added in the panel driver * incorporated width_mm and height_mm suggested by rob herring
v5: * theirry review comments incorporated panel model naming consistent, alphabetical soring in Kconfig Makefile, MAX_BRIGHTNESS dropped, regulator_names, parameterize panel width and height, descprition for control display, cabc and interface setting, temporary variable removed, consistent error reporting and commit message * removed tear on/off, scanline, since these are required only for command mode panels
v6: * emil review comments incorporated PANEL_NUM_REGULATORS dropped, return ret added at necessary places, if checks dropped for backlight and gpios --- drivers/gpu/drm/panel/Kconfig | 11 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 494 +++++++++++++++++++++++++ 3 files changed, 506 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 1500ab9..62aba97 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -18,6 +18,17 @@ config DRM_PANEL_SIMPLE that it can be automatically turned off when the panel goes into a low power state.
+config DRM_PANEL_JDI_LT070ME05000 + tristate "JDI LT070ME05000 WUXGA DSI panel" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for JDI DSI video mode + panel as found in Google Nexus 7 (2013) devices. + The panel has a 1200(RGB)×1920 (WUXGA) resolution and uses + 24 bit per pixel. + config DRM_PANEL_SAMSUNG_LD9040 tristate "Samsung LD9040 RGB/SPI panel" depends on OF && SPI diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index f277eed..a5c7ec0 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -1,4 +1,5 @@ obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o +obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file mode 100644 index 0000000..3213025 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c @@ -0,0 +1,494 @@ +/* + * Copyright (C) 2016 InforceComputing + * Author: Vinay Simha BN simhavcs@gmail.com + * + * Copyright (C) 2016 Linaro Ltd + * Author: Sumit Semwal sumit.semwal@linaro.org + * + * From internet archives, the panel for Nexus 7 2nd Gen, 2013 model is a + * JDI model LT070ME05000, and its data sheet is at: + * http://panelone.net/en/7-0-inch/JDI_LT070ME05000_7.0_inch-datasheet + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see http://www.gnu.org/licenses/. + */ +#include <linux/backlight.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h> + +#include <drm/drmP.h> +#include <drm/drm_crtc.h> +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_panel.h> + +#include <video/mipi_display.h> + +static const char * const regulator_names[] = { + "vddp", + "dcdc_en", + "vcc" +}; + +struct jdi_panel { + struct drm_panel base; + struct mipi_dsi_device *dsi; + + struct regulator_bulk_data supplies[3]; + + struct gpio_desc *reset_gpio; + struct gpio_desc *enable_gpio; + struct backlight_device *backlight; + + bool prepared; + bool enabled; + + const struct drm_display_mode *mode; +}; + +static inline struct jdi_panel *to_jdi_panel(struct drm_panel *panel) +{ + return container_of(panel, struct jdi_panel, base); +} + +static int jdi_panel_init(struct jdi_panel *jdi) +{ + struct mipi_dsi_device *dsi = jdi->dsi; + int ret; + + dsi->mode_flags |= MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_dcs_soft_reset(dsi); + if (ret < 0) + return ret; + + usleep_range(10000, 20000); + + ret = mipi_dsi_dcs_set_pixel_format(dsi, MIPI_DCS_PIXEL_FMT_24BIT << 4); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_set_column_address(dsi, 0, jdi->mode->hdisplay - 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_set_page_address(dsi, 0, jdi->mode->vdisplay - 1); + if (ret < 0) + return ret; + + /* + * BIT(5) BCTRL = 1 Backlight Control Block On, Brightness registers + * are active + * BIT(3) BL = 1 Backlight Control On + * BIT(2) DD = 0 Display Dimming is Off + */ + ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY, + (u8[]){ 0x24 }, 1); + if (ret < 0) + return ret; + + /* CABC off */ + ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_POWER_SAVE, + (u8[]){ 0x00 }, 1); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_exit_sleep_mode(dsi); + if (ret < 0) + return ret; + + msleep(120); + + ret = mipi_dsi_generic_write(dsi, (u8[]){0xB0, 0x00}, 2); + if (ret < 0) + return ret; + + mdelay(10); + + /* Interface setting, video mode */ + ret = mipi_dsi_generic_write(dsi, (u8[]) + {0xB3, 0x26, 0x08, 0x00, 0x20, 0x00}, 6); + if (ret < 0) + return ret; + + mdelay(20); + + ret = mipi_dsi_generic_write(dsi, (u8[]){0xB0, 0x03}, 2); + + return ret; +} + +static int jdi_panel_on(struct jdi_panel *jdi) +{ + struct mipi_dsi_device *dsi = jdi->dsi; + int ret; + + dsi->mode_flags |= MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_dcs_set_display_on(dsi); + + return ret; +} + +static int jdi_panel_off(struct jdi_panel *jdi) +{ + struct mipi_dsi_device *dsi = jdi->dsi; + int ret; + + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_dcs_set_display_off(dsi); + if (ret < 0) + return ret; + + ret = mipi_dsi_dcs_enter_sleep_mode(dsi); + + msleep(100); + + return ret; +} + +static int jdi_panel_disable(struct drm_panel *panel) +{ + struct jdi_panel *jdi = to_jdi_panel(panel); + + if (!jdi->enabled) + return 0; + + jdi->backlight->props.power = FB_BLANK_POWERDOWN; + backlight_update_status(jdi->backlight); + + jdi->enabled = false; + + return 0; +} + +static int jdi_panel_unprepare(struct drm_panel *panel) +{ + struct jdi_panel *jdi = to_jdi_panel(panel); + struct device *dev = &jdi->dsi->dev; + int ret; + + if (!jdi->prepared) + return 0; + + ret = jdi_panel_off(jdi); + if (ret) { + dev_err(panel->dev, "failed to set panel off: %d\n", ret); + return ret; + } + + ret = regulator_bulk_disable(ARRAY_SIZE(jdi->supplies), jdi->supplies); + if (ret < 0) + dev_err(dev, "regulator disable failed, %d\n", ret); + + gpiod_set_value(jdi->reset_gpio, 0); + + gpiod_set_value(jdi->enable_gpio, 0); + + jdi->prepared = false; + + return ret; +} + +static int jdi_panel_prepare(struct drm_panel *panel) +{ + struct jdi_panel *jdi = to_jdi_panel(panel); + struct device *dev = &jdi->dsi->dev; + int ret; + + if (jdi->prepared) + return 0; + + ret = regulator_bulk_enable(ARRAY_SIZE(jdi->supplies), jdi->supplies); + if (ret < 0) { + dev_err(dev, "regulator enable failed, %d\n", ret); + return ret; + } + + msleep(20); + + if (jdi->reset_gpio) { + gpiod_set_value(jdi->reset_gpio, 1); + usleep_range(10, 20); + } + + if (jdi->enable_gpio) { + gpiod_set_value(jdi->enable_gpio, 1); + usleep_range(10, 20); + } + + ret = jdi_panel_init(jdi); + if (ret < 0) { + dev_err(panel->dev, "failed to init panel: %d\n", ret); + goto poweroff; + } + + ret = jdi_panel_on(jdi); + if (ret < 0) { + dev_err(panel->dev, "failed to set panel on: %d\n", ret); + goto poweroff; + } + + jdi->prepared = true; + + return 0; + +poweroff: + gpiod_set_value(jdi->enable_gpio, 0); + gpiod_set_value(jdi->reset_gpio, 0); + + return ret; +} + +static int jdi_panel_enable(struct drm_panel *panel) +{ + struct jdi_panel *jdi = to_jdi_panel(panel); + + if (jdi->enabled) + return 0; + + jdi->backlight->props.power = FB_BLANK_UNBLANK; + backlight_update_status(jdi->backlight); + + jdi->enabled = true; + + return 0; +} + +static const struct drm_display_mode default_mode = { + .clock = 155493, + .hdisplay = 1200, + .hsync_start = 1200 + 48, + .hsync_end = 1200 + 48 + 32, + .htotal = 1200 + 48 + 32 + 60, + .vdisplay = 1920, + .vsync_start = 1920 + 3, + .vsync_end = 1920 + 3 + 5, + .vtotal = 1920 + 3 + 5 + 6, + .vrefresh = 60, + .flags = 0, +}; + +static int jdi_panel_get_modes(struct drm_panel *panel) +{ + struct drm_display_mode *mode; + struct jdi_panel *jdi = to_jdi_panel(panel); + struct device *dev = &jdi->dsi->dev; + + mode = drm_mode_duplicate(panel->drm, &default_mode); + if (!mode) { + dev_err(dev, "failed to add mode %ux%ux@%u\n", + default_mode.hdisplay, default_mode.vdisplay, + default_mode.vrefresh); + return -ENOMEM; + } + + drm_mode_set_name(mode); + + drm_mode_probed_add(panel->connector, mode); + + panel->connector->display_info.width_mm = 95; + panel->connector->display_info.height_mm = 151; + + return 1; +} + +static int dsi_dcs_bl_get_brightness(struct backlight_device *bl) +{ + struct mipi_dsi_device *dsi = bl_get_data(bl); + int ret; + u16 brightness = bl->props.brightness; + + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness); + if (ret < 0) + return ret; + + dsi->mode_flags |= MIPI_DSI_MODE_LPM; + + return brightness & 0xff; +} + +static int dsi_dcs_bl_update_status(struct backlight_device *bl) +{ + struct mipi_dsi_device *dsi = bl_get_data(bl); + int ret; + + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; + + ret = mipi_dsi_dcs_set_display_brightness(dsi, bl->props.brightness); + if (ret < 0) + return ret; + + dsi->mode_flags |= MIPI_DSI_MODE_LPM; + + return 0; +} + +static const struct backlight_ops dsi_bl_ops = { + .update_status = dsi_dcs_bl_update_status, + .get_brightness = dsi_dcs_bl_get_brightness, +}; + +struct backlight_device * +drm_panel_create_dsi_backlight(struct mipi_dsi_device *dsi) +{ + struct device *dev = &dsi->dev; + struct backlight_properties props; + + memset(&props, 0, sizeof(props)); + props.type = BACKLIGHT_RAW; + props.brightness = 255; + props.max_brightness = 255; + + return devm_backlight_device_register(dev, dev_name(dev), dev, dsi, + &dsi_bl_ops, &props); +} + +static const struct drm_panel_funcs jdi_panel_funcs = { + .disable = jdi_panel_disable, + .unprepare = jdi_panel_unprepare, + .prepare = jdi_panel_prepare, + .enable = jdi_panel_enable, + .get_modes = jdi_panel_get_modes, +}; + +static const struct of_device_id jdi_of_match[] = { + { .compatible = "jdi,lt070me05000", }, + { } +}; +MODULE_DEVICE_TABLE(of, jdi_of_match); + +static int jdi_panel_add(struct jdi_panel *jdi) +{ + struct device *dev = &jdi->dsi->dev; + int ret; + unsigned int i; + + jdi->mode = &default_mode; + + for (i = 0; i < ARRAY_SIZE(jdi->supplies); i++) + jdi->supplies[i].supply = regulator_names[i]; + + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(jdi->supplies), + jdi->supplies); + if (ret < 0) { + dev_err(dev, "failed to init regulator, ret=%d\n", ret); + return ret; + } + + jdi->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(jdi->reset_gpio)) { + ret = PTR_ERR(jdi->reset_gpio); + dev_err(dev, "cannot get reset-gpios %d\n", ret); + return ret; + } + + jdi->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW); + if (IS_ERR(jdi->enable_gpio)) { + ret = PTR_ERR(jdi->enable_gpio); + dev_err(dev, "cannot get enable-gpio %d\n", ret); + return ret; + } + + jdi->backlight = drm_panel_create_dsi_backlight(jdi->dsi); + if (IS_ERR(jdi->backlight)) { + ret = PTR_ERR(jdi->backlight); + dev_err(dev, "failed to register backlight %d\n", ret); + return ret; + } + + drm_panel_init(&jdi->base); + jdi->base.funcs = &jdi_panel_funcs; + jdi->base.dev = &jdi->dsi->dev; + + ret = drm_panel_add(&jdi->base); + + return ret; +} + +static void jdi_panel_del(struct jdi_panel *jdi) +{ + if (jdi->base.dev) + drm_panel_remove(&jdi->base); +} + +static int jdi_panel_probe(struct mipi_dsi_device *dsi) +{ + struct jdi_panel *jdi; + int ret; + + dsi->lanes = 4; + dsi->format = MIPI_DSI_FMT_RGB888; + dsi->mode_flags = MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO | + MIPI_DSI_CLOCK_NON_CONTINUOUS; + + jdi = devm_kzalloc(&dsi->dev, sizeof(*jdi), GFP_KERNEL); + if (!jdi) + return -ENOMEM; + + mipi_dsi_set_drvdata(dsi, jdi); + + jdi->dsi = dsi; + + ret = jdi_panel_add(jdi); + if (ret < 0) + return ret; + + return mipi_dsi_attach(dsi); +} + +static int jdi_panel_remove(struct mipi_dsi_device *dsi) +{ + struct jdi_panel *jdi = mipi_dsi_get_drvdata(dsi); + int ret; + + ret = jdi_panel_disable(&jdi->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); + + drm_panel_detach(&jdi->base); + jdi_panel_del(jdi); + + return 0; +} + +static void jdi_panel_shutdown(struct mipi_dsi_device *dsi) +{ + struct jdi_panel *jdi = mipi_dsi_get_drvdata(dsi); + + jdi_panel_disable(&jdi->base); +} + +static struct mipi_dsi_driver jdi_panel_driver = { + .driver = { + .name = "panel-jdi-lt070me05000", + .of_match_table = jdi_of_match, + }, + .probe = jdi_panel_probe, + .remove = jdi_panel_remove, + .shutdown = jdi_panel_shutdown, +}; +module_mipi_dsi_driver(jdi_panel_driver); + +MODULE_AUTHOR("Sumit Semwal sumit.semwal@linaro.org"); +MODULE_AUTHOR("Vinay Simha BN simhavcs@gmail.com"); +MODULE_DESCRIPTION("JDI LT070ME05000 WUXGA"); +MODULE_LICENSE("GPL v2");
Hi Vinay,
On 17 June 2016 at 19:23, Vinay Simha BN simhavcs@gmail.com wrote:
v6:
- emil review comments incorporated PANEL_NUM_REGULATORS dropped, return ret added at necessary places, if checks dropped for backlight and gpios
Looks like some of my suggestions went below the radar. Did you miss them or I've I lost the plot somewhere ? In case of the latter don't be shy to point out :-)
+struct jdi_panel {
struct drm_panel base;
struct mipi_dsi_device *dsi;
struct regulator_bulk_data supplies[3];
struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
+static int jdi_panel_off(struct jdi_panel *jdi) +{
struct mipi_dsi_device *dsi = jdi->dsi;
int ret;
dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
ret = mipi_dsi_dcs_set_display_off(dsi);
if (ret < 0)
return ret;
IMHO neither this function nor its caller jdi_panel_unprepare() should stop in the middle/return prematurely.
Or in other words - one should change the function return type to void and drop the returns.
+static int jdi_panel_unprepare(struct drm_panel *panel) +{
struct jdi_panel *jdi = to_jdi_panel(panel);
struct device *dev = &jdi->dsi->dev;
int ret;
if (!jdi->prepared)
return 0;
ret = jdi_panel_off(jdi);
if (ret) {
dev_err(panel->dev, "failed to set panel off: %d\n", ret);
return ret;
As suggested above, drop this return.
+static int jdi_panel_prepare(struct drm_panel *panel) +{
struct jdi_panel *jdi = to_jdi_panel(panel);
struct device *dev = &jdi->dsi->dev;
int ret;
if (jdi->prepared)
return 0;
ret = regulator_bulk_enable(ARRAY_SIZE(jdi->supplies), jdi->supplies);
if (ret < 0) {
dev_err(dev, "regulator enable failed, %d\n", ret);
return ret;
}
msleep(20);
if (jdi->reset_gpio) {
You can drop the check.
gpiod_set_value(jdi->reset_gpio, 1);
usleep_range(10, 20);
}
if (jdi->enable_gpio) {
Ditto.
+poweroff:
gpiod_set_value(jdi->enable_gpio, 0);
gpiod_set_value(jdi->reset_gpio, 0);
Just noticed that you're missing regulator_bulk_disable() here.
Regards, Emil
On Sat, Jun 18, 2016 at 5:58 AM, Emil Velikov emil.l.velikov@gmail.com wrote:
Hi Vinay,
On 17 June 2016 at 19:23, Vinay Simha BN simhavcs@gmail.com wrote:
v6:
- emil review comments incorporated PANEL_NUM_REGULATORS dropped, return ret added at necessary places, if checks dropped for backlight and gpios
Looks like some of my suggestions went below the radar. Did you miss them or I've I lost the plot somewhere ? In case of the latter don't be shy to point out :-)
+struct jdi_panel {
struct drm_panel base;
struct mipi_dsi_device *dsi;
struct regulator_bulk_data supplies[3];
struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
+static int jdi_panel_off(struct jdi_panel *jdi) +{
struct mipi_dsi_device *dsi = jdi->dsi;
int ret;
dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
ret = mipi_dsi_dcs_set_display_off(dsi);
if (ret < 0)
return ret;
IMHO neither this function nor its caller jdi_panel_unprepare() should stop in the middle/return prematurely.
Or in other words - one should change the function return type to void and drop the returns.
+static int jdi_panel_unprepare(struct drm_panel *panel) +{
struct jdi_panel *jdi = to_jdi_panel(panel);
struct device *dev = &jdi->dsi->dev;
int ret;
if (!jdi->prepared)
return 0;
ret = jdi_panel_off(jdi);
if (ret) {
dev_err(panel->dev, "failed to set panel off: %d\n", ret);
return ret;
As suggested above, drop this return.
i can make the function void for jdi_panel_off and drop the return, but i cannot make void for jdi_panel_unprepare, since drm_panel_prepare requires 0 or negative value.
* call to drm_panel_prepare(). * * Return: 0 on success or a negative error code on failure. */ static inline int drm_panel_unprepare(struct drm_panel *panel) { if (panel && panel->funcs && panel->funcs->unprepare) return panel->funcs->unprepare(panel);
return panel ? -ENOSYS : -EINVAL; }
+static int jdi_panel_prepare(struct drm_panel *panel) +{
struct jdi_panel *jdi = to_jdi_panel(panel);
struct device *dev = &jdi->dsi->dev;
int ret;
if (jdi->prepared)
return 0;
ret = regulator_bulk_enable(ARRAY_SIZE(jdi->supplies), jdi->supplies);
if (ret < 0) {
dev_err(dev, "regulator enable failed, %d\n", ret);
return ret;
}
msleep(20);
if (jdi->reset_gpio) {
You can drop the check.
gpiod_set_value(jdi->reset_gpio, 1);
usleep_range(10, 20);
}
if (jdi->enable_gpio) {
Ditto.
+poweroff:
gpiod_set_value(jdi->enable_gpio, 0);
gpiod_set_value(jdi->reset_gpio, 0);
Just noticed that you're missing regulator_bulk_disable() here.
Regards, Emil
On 18 June 2016 at 07:59, Vinay Simha simhavcs@gmail.com wrote:
On Sat, Jun 18, 2016 at 5:58 AM, Emil Velikov emil.l.velikov@gmail.com wrote:
Hi Vinay,
On 17 June 2016 at 19:23, Vinay Simha BN simhavcs@gmail.com wrote:
v6:
- emil review comments incorporated PANEL_NUM_REGULATORS dropped, return ret added at necessary places, if checks dropped for backlight and gpios
Looks like some of my suggestions went below the radar. Did you miss them or I've I lost the plot somewhere ? In case of the latter don't be shy to point out :-)
+struct jdi_panel {
struct drm_panel base;
struct mipi_dsi_device *dsi;
struct regulator_bulk_data supplies[3];
struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
+static int jdi_panel_off(struct jdi_panel *jdi) +{
struct mipi_dsi_device *dsi = jdi->dsi;
int ret;
dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
ret = mipi_dsi_dcs_set_display_off(dsi);
if (ret < 0)
return ret;
IMHO neither this function nor its caller jdi_panel_unprepare() should stop in the middle/return prematurely.
Or in other words - one should change the function return type to void and drop the returns.
+static int jdi_panel_unprepare(struct drm_panel *panel) +{
struct jdi_panel *jdi = to_jdi_panel(panel);
struct device *dev = &jdi->dsi->dev;
int ret;
if (!jdi->prepared)
return 0;
ret = jdi_panel_off(jdi);
if (ret) {
dev_err(panel->dev, "failed to set panel off: %d\n", ret);
return ret;
As suggested above, drop this return.
i can make the function void for jdi_panel_off and drop the return, but i cannot make void for jdi_panel_unprepare, since drm_panel_prepare requires 0 or negative value.
Seems like I wasn't clear enough - all you want here is to drop the spurious return.
Regards, Emil
dri-devel@lists.freedesktop.org