Skip to contentSkip to conversation

drm/dsi: Implement set tear scanline

PATCH Superseded Vinay Simha BN 2 participants 6 messages Jun 2, 2016 – Jun 6, 2016

Subsystems (2)
DRM drivers DRM misc

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Vinay Simha BN

author
#0 lore raw

PATCH 1/2 drm/dsi: Implement set tear scanline

Provide a small convenience wrapper that transmits ia set_tear_scanline command as suggested by Thierry Reding.

Also includes small build fixes from Sumit Semwal.

Cc Archit Taneja <[email protected]>
Cc John Stultz <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Sumit Semwal <[email protected]>
Signed-off-by Vinay Simha BN <[email protected]>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 23 +++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h     |  2 ++
 2 files changed, 25 insertions(+)
drivers/gpu/drm/drm_mipi_dsi.c +23 -0
index f5d8083..2f0b85c 100644
@@ -983,6 +983,29 @@ int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
 EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on);
 
 /**
+ * mipi_dsi_set_tear_scanline() - turn on the display module's Tearing Effect
+ * output signal on the TE signal line when display module reaches line N
+ * defined by STS[n:0].
+ * @dsi: DSI peripheral device
+ * @param1: STS[10:8]
+ * @param2: STS[7:0]
+ * Return: 0 on success or a negative error code on failure
+ */
+int mipi_dsi_set_tear_scanline(struct mipi_dsi_device *dsi,
+			       u8 param1, u8 param2)
+{
+	u8 payload[3] = { MIPI_DCS_SET_TEAR_SCANLINE, param1, param2};
+	ssize_t err;
+
+	err = mipi_dsi_generic_write(dsi, &payload, sizeof(payload));
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+EXPORT_SYMBOL(mipi_dsi_set_tear_scanline);
+
+/**
  * mipi_dsi_dcs_set_pixel_format() - sets the pixel format for the RGB image
  *    data used by the interface
  * @dsi: DSI peripheral device
include/drm/drm_mipi_dsi.h +2 -0
index 7a9840f..2788dbe 100644
@@ -263,6 +263,8 @@ int mipi_dsi_dcs_set_column_address(struct mipi_dsi_device *dsi, u16 start,
 				    u16 end);
 int mipi_dsi_dcs_set_page_address(struct mipi_dsi_device *dsi, u16 start,
 				  u16 end);
+int mipi_dsi_set_tear_scanline(struct mipi_dsi_device *dsi, u8 param1,
+			       u8 param2);
 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);
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 2/2 drm/dsi: Implement dcs backlight brightness

Provide a small convenience wrapper that set/get the backlight brightness control and creates the backlight device for the panel interface

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

--
v1:
*tested in nexus7 2nd gen.

---
 drivers/gpu/drm/drm_mipi_dsi.c | 63 ++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h     |  3 ++
 2 files changed, 66 insertions(+)
drivers/gpu/drm/drm_mipi_dsi.c +63 -0
index 2f0b85c..ac4521e 100644
@@ -1026,6 +1026,69 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format)
 }
 EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
 
+static int dsi_dcs_bl_get_brightness(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	int ret;
+	u8 brightness;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS,
+				&brightness, sizeof(brightness));
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	return brightness;
+}
+
+static int dsi_dcs_bl_update_status(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	int ret;
+	u8 brightness = bl->props.brightness;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
+				 &brightness, sizeof(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,
+};
+
+/**
+ * drm_panel_create_dsi_backlight() - creates the backlight device for the panel
+ * @dsi: DSI peripheral device
+ *
+ * @return a struct backlight on success, or an ERR_PTR on error
+ */
+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);
+}
+EXPORT_SYMBOL(drm_panel_create_dsi_backlight);
+
 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 +3 -0
index 2788dbe..9dd6a97 100644
@@ -12,6 +12,7 @@
 #ifndef __DRM_MIPI_DSI_H__
 #define __DRM_MIPI_DSI_H__
 
+#include <linux/backlight.h>
 #include <linux/device.h>
 
 struct mipi_dsi_host;

@@ -269,6 +270,8 @@ 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);
+struct backlight_device
+	*drm_panel_create_dsi_backlight(struct mipi_dsi_device *dsi);
 
 /**
  * struct mipi_dsi_driver - DSI driver
Signature · 1 line
1.9.1
#2 lore raw

Re: [PATCH 2/2] drm/dsi: Implement dcs backlight brightness

Quoted 3 lines · Vinay Simha BN

Provide a small convenience wrapper that set/get the backlight brightness control and creates the backlight device for the panel interface

To be pedantic, we should downplay "backlight" in the DSI DCS brightness control... there need not be a backlight, at all, for brightness control (see AMOLED).

Quoted 19 lines
Cc John Stultz <[email protected]>
Cc Sumit Semwal <[email protected]>
Cc Archit Taneja <[email protected]>
Cc Rob Clark <[email protected]>
Signed-off-by Vinay Simha BN <[email protected]>

--
v1:
*tested in nexus7 2nd gen.

---
 drivers/gpu/drm/drm_mipi_dsi.c | 63 ++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h     |  3 ++
 2 files changed, 66 insertions(+)
drivers/gpu/drm/drm_mipi_dsi.c +0 -0
index 2f0b85c..ac4521e 100644

drivers/gpu/drm/drm_mipi_dsi.c int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) · line 1029

Quoted 4 lines
drivers/gpu/drm/drm_mipi_dsi.c +1 -0
@@ -1026,6 +1026,69 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format)
 }
 EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
 
+static int dsi_dcs_bl_get_brightness(struct backlight_device *bl)

I'd rather see convenience helpers that are not tied to struct backlight_device. You can add a one-line wrapper on top that takes struct backlight_device pointer.

While at it, please name the helpers according to the DCS command.

Quoted 46 lines
drivers/gpu/drm/drm_mipi_dsi.c +46 -0
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	int ret;
+	u8 brightness;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS,
+				&brightness, sizeof(brightness));
+	if (ret < 0)
+		return ret;
+
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	return brightness;
+}
+
+static int dsi_dcs_bl_update_status(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	int ret;
+	u8 brightness = bl->props.brightness;
+
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
+				 &brightness, sizeof(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,
+};
+
+/**
+ * drm_panel_create_dsi_backlight() - creates the backlight device for the panel
+ * @dsi: DSI peripheral device
+ *
+ * @return a struct backlight on success, or an ERR_PTR on error
+ */

drivers/gpu/drm/drm_mipi_dsi.c int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) · line 1085

Quoted 10 lines
drivers/gpu/drm/drm_mipi_dsi.c +10 -0
+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;

The DCS spec says the max is either 0xff or 0xffff depending on the implementation... this obviously affects the helpers as well.

And how about the backlight bits in write_control_display? I just fear this is a simplistic view of brightness control on DSI, and this will grow hairy over time. I think I'd rather see generic DSI DCS brightness *helpers* in this file, and then *perhaps* a separate file for brightness control in general.

BR,
Jani.

Quoted 30 lines
drivers/gpu/drm/drm_mipi_dsi.c +6 -0
+
+	return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+					      &dsi_bl_ops, &props);
+}
+EXPORT_SYMBOL(drm_panel_create_dsi_backlight);
+
 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 +3 -0
index 2788dbe..9dd6a97 100644
@@ -12,6 +12,7 @@
 #ifndef __DRM_MIPI_DSI_H__
 #define __DRM_MIPI_DSI_H__
 
+#include <linux/backlight.h>
 #include <linux/device.h>
 
 struct mipi_dsi_host;

@@ -269,6 +270,8 @@ 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);
+struct backlight_device
+	*drm_panel_create_dsi_backlight(struct mipi_dsi_device *dsi);
 
 /**
  * struct mipi_dsi_driver - DSI driver
Signature · 1 line
Jani Nikula, Intel Open Source Technology Center
author
#3 lore raw

Re: [PATCH 2/2] drm/dsi: Implement dcs backlight brightness

Quoted 8 lines · Jani Nikula
Quoted 3 lines · Vinay Simha BN

Provide a small convenience wrapper that set/get the backlight brightness control and creates the backlight device for the panel interface

To be pedantic, we should downplay "backlight" in the DSI DCS brightness control... there need not be a backlight, at all, for brightness control (see AMOLED).

but this jdi display and few other dsi display can be controlled by dcs commands

Quoted 23 lines
Quoted 23 lines
Cc John Stultz <[email protected]>
Cc Sumit Semwal <[email protected]>
Cc Archit Taneja <[email protected]>
Cc Rob Clark <[email protected]>
Signed-off-by Vinay Simha BN <[email protected]>

--
v1:
*tested in nexus7 2nd gen.

---
 drivers/gpu/drm/drm_mipi_dsi.c | 63 ++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h     |  3 ++
 2 files changed, 66 insertions(+)
drivers/gpu/drm/drm_mipi_dsi.c +0 -0
index 2f0b85c..ac4521e 100644
@@ -1026,6 +1026,69 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format)
 }
 EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
Quoted 6 lines

drivers/gpu/drm/drm_mipi_dsi.c int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) · line 1029

Quoted 1 line
drivers/gpu/drm/drm_mipi_dsi.c +1 -0
+static int dsi_dcs_bl_get_brightness(struct backlight_device *bl)

I'd rather see convenience helpers that are not tied to struct backlight_device. You can add a one-line wrapper on top that takes struct backlight_device pointer.

i need to move the backlight_ops, backlight struct in panel driver and need to set/get brightness in drm_mipi_dsi.c

Quoted 1 line

While at it, please name the helpers according to the DCS command.

i will changes this to in v2 version
mipi_dsi_dcs_set_brightness
mipi_dsi_dcs_get_brightness

Quoted 61 lines
Quoted 47 lines
drivers/gpu/drm/drm_mipi_dsi.c +46 -0
+{
+     struct mipi_dsi_device *dsi = bl_get_data(bl);
+     int ret;
+     u8 brightness;
+
+     dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+     ret = mipi_dsi_dcs_read(dsi, MIPI_DCS_GET_DISPLAY_BRIGHTNESS,
+                             &brightness, sizeof(brightness));
+     if (ret < 0)
+             return ret;
+
+     dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+     return brightness;
+}
+
+static int dsi_dcs_bl_update_status(struct backlight_device *bl)
+{
+     struct mipi_dsi_device *dsi = bl_get_data(bl);
+     int ret;
+     u8 brightness = bl->props.brightness;
+
+     dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+     ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_BRIGHTNESS,
+                              &brightness, sizeof(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,
+};
+
+/**
+ * drm_panel_create_dsi_backlight() - creates the backlight device for the panel
+ * @dsi: DSI peripheral device
+ *
+ * @return a struct backlight on success, or an ERR_PTR on error
+ */

drivers/gpu/drm/drm_mipi_dsi.c int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) · line 1085

Quoted 10 lines
drivers/gpu/drm/drm_mipi_dsi.c +10 -0
+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;

The DCS spec says the max is either 0xff or 0xffff depending on the implementation... this obviously affects the helpers as well.

Quoted 1 line

And how about the backlight bits in write_control_display? 8 bits

I just fear

Quoted 41 lines

this is a simplistic view of brightness control on DSI, and this will grow hairy over time. I think I'd rather see generic DSI DCS brightness *helpers* in this file, and then *perhaps* a separate file for brightness control in general.

BR,
Jani.

Quoted 22 lines
drivers/gpu/drm/drm_mipi_dsi.c +6 -0
+
+     return devm_backlight_device_register(dev, dev_name(dev), dev, dsi,
+                                           &dsi_bl_ops, &props);
+}
+EXPORT_SYMBOL(drm_panel_create_dsi_backlight);
+
 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 +1 -0
index 2788dbe..9dd6a97 100644
@@ -12,6 +12,7 @@
 #ifndef __DRM_MIPI_DSI_H__
 #define __DRM_MIPI_DSI_H__
 
+#include <linux/backlight.h>
 #include <linux/device.h>
 
 struct mipi_dsi_host;

include/drm/drm_mipi_dsi.h int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi); · line 277

Quoted 8 lines
include/drm/drm_mipi_dsi.h +2 -0
@@ -269,6 +270,8 @@ 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);
+struct backlight_device
+     *drm_panel_create_dsi_backlight(struct mipi_dsi_device *dsi);
 
 /**
  * struct mipi_dsi_driver - DSI driver

--
Jani Nikula, Intel Open Source Technology Center

Signature · 3 lines
Regards,

Vinay Simha.B.N.
#4 lore raw

Re: [PATCH 2/2] drm/dsi: Implement dcs backlight brightness

Quoted 11 lines · Vinay Simha
Quoted 8 lines · Jani Nikula
Quoted 3 lines · Vinay Simha BN

Provide a small convenience wrapper that set/get the backlight brightness control and creates the backlight device for the panel interface

To be pedantic, we should downplay "backlight" in the DSI DCS brightness control... there need not be a backlight, at all, for brightness control (see AMOLED).

but this jdi display and few other dsi display can be controlled by dcs commands

The point I was trying to convey was that the DSI DCS interface is agnostic to the actual method of controlling brightness. Whether the panel has a backlight to control brightness is a panel implementation detail. So maybe we shouldn't have "backlight" in the interface nomenclature either.

BR,
Jani.

Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#5 lore raw
In reply to Vinay Simha BN
Quoted 24 lines · Vinay Simha BN

Provide a small convenience wrapper that transmits ia set_tear_scanline command as suggested by Thierry Reding.

Also includes small build fixes from Sumit Semwal.

Cc Archit Taneja <[email protected]>
Cc John Stultz <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Sumit Semwal <[email protected]>
Signed-off-by Vinay Simha BN <[email protected]>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 23 +++++++++++++++++++++++
 include/drm/drm_mipi_dsi.h     |  2 ++
 2 files changed, 25 insertions(+)
drivers/gpu/drm/drm_mipi_dsi.c +0 -0
index f5d8083..2f0b85c 100644
@@ -983,6 +983,29 @@ int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
 EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on);
 
 /**

drivers/gpu/drm/drm_mipi_dsi.c int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi, · line 995

Quoted 10 lines
drivers/gpu/drm/drm_mipi_dsi.c +10 -0
+ * mipi_dsi_set_tear_scanline() - turn on the display module's Tearing Effect
+ * output signal on the TE signal line when display module reaches line N
+ * defined by STS[n:0].
+ * @dsi: DSI peripheral device
+ * @param1: STS[10:8]
+ * @param2: STS[7:0]
+ * Return: 0 on success or a negative error code on failure
+ */
+int mipi_dsi_set_tear_scanline(struct mipi_dsi_device *dsi,
+			       u8 param1, u8 param2)

If you're making a helper, please make using it easy and have *one* scanline parameter.

BR,
Jani.

Quoted 29 lines
drivers/gpu/drm/drm_mipi_dsi.c +13 -0
+{
+	u8 payload[3] = { MIPI_DCS_SET_TEAR_SCANLINE, param1, param2};
+	ssize_t err;
+
+	err = mipi_dsi_generic_write(dsi, &payload, sizeof(payload));
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+EXPORT_SYMBOL(mipi_dsi_set_tear_scanline);
+
+/**
  * mipi_dsi_dcs_set_pixel_format() - sets the pixel format for the RGB image
  *    data used by the interface
  * @dsi: DSI peripheral device
include/drm/drm_mipi_dsi.h +2 -0
index 7a9840f..2788dbe 100644
@@ -263,6 +263,8 @@ int mipi_dsi_dcs_set_column_address(struct mipi_dsi_device *dsi, u16 start,
 				    u16 end);
 int mipi_dsi_dcs_set_page_address(struct mipi_dsi_device *dsi, u16 start,
 				  u16 end);
+int mipi_dsi_set_tear_scanline(struct mipi_dsi_device *dsi, u8 param1,
+			       u8 param2);
 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);
Signature · 1 line
Jani Nikula, Intel Open Source Technology Center