Skip to contentSkip to conversation

drm/dsi: Implement dcs set/get display brightness

PATCH Superseded Vinay Simha BN 2 participants 5 messages Jun 17, 2016 – Jun 18, 2016

Subsystems (3)
DRM drivers DRM misc DRM panel drivers

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Vinay Simha BN

author
#0 lore raw

PATCH v2 1/2 drm/dsi: Implement dcs set/get display brightness

Provide a small convenience wrapper that set/get the display brightness value

Cc John Stultz <[email protected]>
Cc Sumit Semwal <[email protected]>
Cc Archit Taneja <[email protected]>
Cc Rob Clark <[email protected]>
Cc Jani Nikula <[email protected]>
Cc Thierry Reding <[email protected]>
Signed-off-by Vinay Simha BN <[email protected]>

---
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(+)
drivers/gpu/drm/drm_mipi_dsi.c +49 -0
index 49311fc..2c03784 100644
@@ -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);
include/drm/drm_mipi_dsi.h +4 -0
index 72f5b15..4d77bb0 100644
@@ -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
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v6 2/2 drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

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 <[email protected]>
Cc Rob Clark <[email protected]>
Cc Sumit Semwal <[email protected]>
Cc John Stultz <[email protected]>
Cc Emil Velikov <[email protected]>
Cc Thierry Reding <[email protected]>
Cc David Airlie <[email protected]>
Signed-off-by Sumit Semwal <[email protected]>
Signed-off-by John Stultz <[email protected]>
Signed-off-by Vinay Simha BN <[email protected]>

---
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
drivers/gpu/drm/panel/Kconfig +11 -0
index 1500ab9..62aba97 100644
@@ -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
drivers/gpu/drm/panel/Makefile +1 -0
index f277eed..a5c7ec0 100644
@@ -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
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +494 -0

494 lines not on this page · raw message

Signature · 1 line
1.9.1
#2 lore raw

Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

Hi Vinay,

Quoted 4 lines · Vinay Simha BN

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 :-)

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 48

Quoted 6 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +6 -0
+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)];

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 153

Quoted 11 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +11 -0
+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.

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 188

Quoted 13 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +13 -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;

As suggested above, drop this return.

Quoted 8 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +8 -0
+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;

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 221

Quoted 10 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +10 -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.

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 226

Quoted 5 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +5 -0
+               gpiod_set_value(jdi->reset_gpio, 1);
+               usleep_range(10, 20);
+       }
+
+       if (jdi->enable_gpio) {

Ditto.

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 250

Quoted 5 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +5 -0
+
+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
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author
#3 lore raw

Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

Quoted 41 lines · Emil Velikov

Hi Vinay,

Quoted 4 lines · Vinay Simha BN

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 :-)

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 48

Quoted 6 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +6 -0
+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)];

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 153

Quoted 11 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +11 -0
+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.

Quoted 15 lines

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 188

Quoted 13 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +13 -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;

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;
}
Quoted 40 lines
Quoted 10 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +8 -0
+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;

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 221

Quoted 10 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +10 -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.

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 226

Quoted 5 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +5 -0
+               gpiod_set_value(jdi->reset_gpio, 1);
+               usleep_range(10, 20);
+       }
+
+       if (jdi->enable_gpio) {

Ditto.

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 250

Quoted 5 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +5 -0
+
+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

Signature · 3 lines
Regards,

Vinay Simha.B.N.
#4 lore raw

Re: [PATCH v6 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

Quoted 57 lines · Vinay Simha
Quoted 41 lines · Emil Velikov

Hi Vinay,

Quoted 4 lines · Vinay Simha BN

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 :-)

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 48

Quoted 6 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +6 -0
+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)];

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 153

Quoted 11 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +11 -0
+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.

Quoted 15 lines

drivers/gpu/drm/panel/panel-jdi-lt070me05000.c line 188

Quoted 13 lines
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c new file +13 -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;

As suggested above, drop this return.

Quoted 4 lines

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 mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel