| Message ID | 1466187806-29465-1-git-send-email-simhavcs@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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(+)