[MIPI CABC 0/2] CABC patch list

[MIPI CABC 0/2] CABC patch list

[Deepak M]

CABC stands for the Content Adaptive Backlight Control.
In the normal display the backlight which we see is due to the
backlight which is being modulated by the filter, which is inturn
dependent on the image. In brief the CABC does the histogram
analysis of the image and then controls the filter and backlight.
For example in CABC to display the dark image the backlight is dimmed
and then controlls the filter to allow more light, because of
which is power consuption will be reduced.

Below are the inital set of patches which supports the CABC.
A field exits in the mipi configuration of the VBT which
when enabled indiactes the CABC is supported. Depending on
this filed the backlight control function pointer are
initalized in the intel_panel.c file.

In case of dual link panels depending on the panel
the DCS commands have to be send to either PORT A,
PORT C or both PORT A and PORT C. Again a filed is
added in the VBT to get this data from the version 197 onwards.
One of the below patches parses these fields from the
VBT.

Addressed the review comments of Jani, which were mentioned in
the below RFC patch
http://lists.freedesktop.org/archives/intel-gfx/2015-September/075819.html

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>

Deepak M (2):
  drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT
  drm/i915: CABC support for backlight control


 drivers/gpu/drm/i915/intel_bios.c  |  13 ++++
 drivers/gpu/drm/i915/intel_bios.h  |   5 +-
 drivers/gpu/drm/i915/intel_dsi.c   |  14 +++-
 drivers/gpu/drm/i915/intel_dsi.h   |  26 +++++++
 drivers/gpu/drm/i915/intel_panel.c | 145 +++++++++++++++++++++++++++++++++++--
 5 files changed, 196 insertions(+), 7 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[MIPI CABC 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT

[Deepak M]

For dual link panel scenarios there are new fileds added in the
VBT which indicate on which port the PWM cntrl and CABC ON/OFF
commands needs to be sent.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
Signed-off-by: Deepak M <m.deepak@intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_bios.h |  5 ++++-
 drivers/gpu/drm/i915/intel_dsi.h  |  2 ++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index ce82f9c..2ef8721 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -793,6 +793,19 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
 		return;
 	}
 
+	/*
+	 * These below bits will inform us on which port the panel blk_cntrl and
+	 * CABC ON/OFF commands needs to be sent in case of dual link panels
+	 *	u16 dl_cabc_port:2;
+	 *	u16 pwm_bkl_ctrl:2;
+	 * But these are introduced from the VBT version 197 onwards, so making
+	 * sure that these bits are zero in the pervious versions.
+	 */
+	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
+		dev_priv->vbt.dsi.config->dl_cabc_port = 0;
+		dev_priv->vbt.dsi.config->pwm_bkl_ctrl = 0;
+	}
+
 	/* We have mandatory mipi config blocks. Initialize as generic panel */
 	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
 
diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
index 7ec8c9a..9283969 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -832,7 +832,10 @@ struct mipi_config {
 	u16 dual_link:2;
 	u16 lane_cnt:2;
 	u16 pixel_overlap:3;
-	u16 rsvd3:9;
+	u16 rgb_flip:1;
+	u16 dl_cabc_port:2;
+	u16 pwm_bkl_ctrl:2;
+	u16 rsvd3:4;
 
 	u16 rsvd4;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index e6cb252..4fde83b 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -74,6 +74,8 @@ struct intel_dsi {
 
 	u8 escape_clk_div;
 	u8 dual_link;
+	u8 bkl_dcs_ports;
+	u8 pwm_blk_ctrl;
 	u8 pixel_overlap;
 	u32 port_bits;
 	u32 bw_timer;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[MIPI CABC 2/2] drm/i915: CABC support for backlight control

[Deepak M]

In CABC (Content Adaptive Brightness Control) content grey level
scale can be increased while simultaneously decreasing
brightness of the backlight to achieve same perceived brightness.

The CABC is not standardized and panel vendors are free to follow
their implementation. The CABC implementaion here assumes that the
panels use standard SW register for control.

In this design there will be no PWM signal from the SoC and DCS
commands are sent to enable and control the backlight brightness.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
Signed-off-by: Deepak M <m.deepak@intel.com>
---
Addressed the review comments from Jani which were mentioned in
the below patch
http://lists.freedesktop.org/archives/intel-gfx/2015-September/075819.html

Went with the name CABC because the commands which are used
here are mainly for CABC and many of the panels have the same
commands for the CABC operation. For the cases where PWM is
directly from panel PWM, DCS commands may be different and
may have to add new functions to support it.

 drivers/gpu/drm/i915/intel_dsi.c   |  14 +++-
 drivers/gpu/drm/i915/intel_dsi.h   |  24 ++++++
 drivers/gpu/drm/i915/intel_panel.c | 145 +++++++++++++++++++++++++++++++++++--
 3 files changed, 177 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 170ae6f..5d1ba35 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1175,8 +1175,20 @@ void intel_dsi_init(struct drm_device *dev)
 		intel_dsi->ports = (1 << PORT_C);
 	}
 
-	if (dev_priv->vbt.dsi.config->dual_link)
+	if (dev_priv->vbt.dsi.config->dual_link) {
 		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
+		switch (dev_priv->vbt.dsi.config->dl_cabc_port) {
+		case CABC_PORT_A:
+			intel_dsi->bkl_dcs_ports = (1 << PORT_A);
+		case CABC_PORT_C:
+			intel_dsi->bkl_dcs_ports = (1 << PORT_C);
+		case CABC_PORT_A_AND_C:
+			intel_dsi->bkl_dcs_ports = intel_dsi->ports;
+		default:
+			DRM_ERROR("Unknown MIPI ports for sending DCS\n");
+		}
+	} else
+		intel_dsi->bkl_dcs_ports = intel_dsi->ports;
 
 	/* Create a DSI host (and a device) for each port. */
 	for_each_dsi_port(port, intel_dsi->ports) {
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 4fde83b..4bcee40 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -34,6 +34,30 @@
 #define DSI_DUAL_LINK_FRONT_BACK	1
 #define DSI_DUAL_LINK_PIXEL_ALT		2
 
+#define CABC_OFF			(0 << 0)
+#define CABC_USER_INTERFACE_IMAGE	(1 << 0)
+#define CABC_STILL_PICTURE		(2 << 0)
+#define CABC_VIDEO_MODE			(3 << 0)
+
+#define CABC_BACKLIGHT			(1 << 2)
+#define CABC_DIMMING_DISPLAY		(1 << 3)
+#define CABC_BCTRL			(1 << 5)
+
+#define CABC_PORT_A			0x00
+#define CABC_PORT_C			0x01
+#define CABC_PORT_A_AND_C		0x02
+
+#define CABC_MAX_VALUE			0xFF
+
+#define MIPI_DCS_CABC_LEVEL_RD		0x52
+#define MIPI_DCS_CABC_MIN_BRIGHTNESS_RD	0x5F
+#define MIPI_DCS_CABC_CONTROL_RD	0x56
+#define MIPI_DCS_CABC_CONTROL_BRIGHT_RD	0x54
+#define MIPI_DCS_CABC_LEVEL_WR		0x51
+#define MIPI_DCS_CABC_MIN_BRIGHTNESS_WR	0x5E
+#define MIPI_DCS_CABC_CONTROL_WR	0x55
+#define MIPI_DCS_CABC_CONTROL_BRIGHT_WR	0x53
+
 struct intel_dsi_host;
 
 struct intel_dsi {
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index a24df35..085d9a6 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -34,6 +34,7 @@
 #include <linux/moduleparam.h>
 #include <linux/pwm.h>
 #include "intel_drv.h"
+#include "intel_dsi.h"
 
 #define CRC_PMIC_PWM_PERIOD_NS	21333
 
@@ -533,6 +534,30 @@ static u32 vlv_get_backlight(struct intel_connector *connector)
 	return _vlv_get_backlight(dev, pipe);
 }
 
+static u32 cabc_get_backlight(struct intel_connector *connector)
+{
+	struct intel_dsi *intel_dsi = NULL;
+	struct intel_encoder *encoder = NULL;
+	struct mipi_dsi_device *dsi_device;
+	u8 data[2] = {0};
+	enum port port;
+
+	encoder = connector->encoder;
+	if (encoder->type == INTEL_OUTPUT_DSI)
+		intel_dsi = enc_to_intel_dsi(&encoder->base);
+	else {
+		DRM_ERROR("Use DSI encoder for CABC\n");
+		return -EINVAL;
+	}
+
+	for_each_dsi_port(port, intel_dsi->bkl_dcs_ports) {
+		dsi_device = intel_dsi->dsi_hosts[port]->device;
+		mipi_dsi_dcs_read(dsi_device, MIPI_DCS_CABC_LEVEL_RD, data, 2);
+	}
+
+	return data[1];
+}
+
 static u32 bxt_get_backlight(struct intel_connector *connector)
 {
 	struct drm_device *dev = connector->base.dev;
@@ -631,6 +656,30 @@ static void vlv_set_backlight(struct intel_connector *connector, u32 level)
 	I915_WRITE(VLV_BLC_PWM_CTL(pipe), tmp | level);
 }
 
+static void cabc_set_backlight(struct intel_connector *connector, u32 level)
+{
+	struct intel_dsi *intel_dsi = NULL;
+	struct intel_encoder *encoder = NULL;
+	struct mipi_dsi_device *dsi_device;
+	u8 data[2] = {0};
+	enum port port;
+
+	encoder = connector->encoder;
+	if (encoder->type == INTEL_OUTPUT_DSI)
+		intel_dsi = enc_to_intel_dsi(&encoder->base);
+	else {
+		DRM_ERROR("Use DSI encoder for CABC\n");
+		return;
+	}
+
+	for_each_dsi_port(port, intel_dsi->bkl_dcs_ports) {
+		dsi_device = intel_dsi->dsi_hosts[port]->device;
+		data[1] = level;
+		data[0] = MIPI_DCS_CABC_LEVEL_WR;
+		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
+	}
+}
+
 static void bxt_set_backlight(struct intel_connector *connector, u32 level)
 {
 	struct drm_device *dev = connector->base.dev;
@@ -798,6 +847,34 @@ static void vlv_disable_backlight(struct intel_connector *connector)
 	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE);
 }
 
+static void cabc_disable_backlight(struct intel_connector *connector)
+{
+	struct intel_dsi *intel_dsi = NULL;
+	struct intel_encoder *encoder = NULL;
+	struct mipi_dsi_device *dsi_device;
+	enum port port;
+	u8 data[2] = {0};
+
+	encoder = connector->encoder;
+	if (encoder->type == INTEL_OUTPUT_DSI)
+		intel_dsi = enc_to_intel_dsi(&encoder->base);
+	else {
+		DRM_ERROR("Use DSI encoder for CABC\n");
+		return;
+	}
+
+	intel_panel_actually_set_backlight(connector, 0);
+
+	for_each_dsi_port(port, intel_dsi->bkl_dcs_ports) {
+		dsi_device = intel_dsi->dsi_hosts[port]->device;
+		data[1] = CABC_OFF;
+		data[0] = MIPI_DCS_CABC_CONTROL_WR;
+		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
+		data[0] = MIPI_DCS_CABC_CONTROL_BRIGHT_WR;
+		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
+	}
+}
+
 static void bxt_disable_backlight(struct intel_connector *connector)
 {
 	struct drm_device *dev = connector->base.dev;
@@ -1133,6 +1210,36 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
 	mutex_unlock(&dev_priv->backlight_lock);
 }
 
+static void cabc_enable_backlight(struct intel_connector *connector)
+{
+	struct intel_dsi *intel_dsi = NULL;
+	struct intel_encoder *encoder = NULL;
+	struct intel_panel *panel = &connector->panel;
+	struct mipi_dsi_device *dsi_device;
+	enum port port;
+	u8 data[2] = {0};
+
+	encoder = connector->encoder;
+	if (encoder->type == INTEL_OUTPUT_DSI)
+		intel_dsi = enc_to_intel_dsi(&encoder->base);
+	else {
+		DRM_ERROR("Use DSI encoder for the CABC\n");
+		return;
+	}
+
+	for_each_dsi_port(port, intel_dsi->bkl_dcs_ports) {
+		dsi_device = intel_dsi->dsi_hosts[port]->device;
+		data[0] = MIPI_DCS_CABC_CONTROL_BRIGHT_WR;
+		data[1] = CABC_BACKLIGHT | CABC_DIMMING_DISPLAY | CABC_BCTRL;
+		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
+		data[0] = MIPI_DCS_CABC_CONTROL_WR;
+		data[1] = CABC_STILL_PICTURE;
+		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
+	}
+
+	intel_panel_actually_set_backlight(connector, panel->backlight.level);
+}
+
 #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
 static int intel_backlight_device_update_status(struct backlight_device *bd)
 {
@@ -1601,6 +1708,26 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe
 	return 0;
 }
 
+static int cabc_setup_backlight(struct intel_connector *connector,
+		enum pipe unused)
+{
+	struct drm_device *dev = connector->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_panel *panel = &connector->panel;
+
+	if (dev_priv->vbt.backlight.present)
+		panel->backlight.present = true;
+	else {
+		DRM_ERROR("no backlight present per VBT\n");
+		return 0;
+	}
+
+	panel->backlight.max = CABC_MAX_VALUE;
+	panel->backlight.level = CABC_MAX_VALUE;
+
+	return 0;
+}
+
 static int
 bxt_setup_backlight(struct intel_connector *connector, enum pipe unused)
 {
@@ -1745,11 +1872,19 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (IS_BROXTON(dev)) {
-		panel->backlight.setup = bxt_setup_backlight;
-		panel->backlight.enable = bxt_enable_backlight;
-		panel->backlight.disable = bxt_disable_backlight;
-		panel->backlight.set = bxt_set_backlight;
-		panel->backlight.get = bxt_get_backlight;
+		if (dev_priv->vbt.dsi.config->cabc_supported) {
+			panel->backlight.setup = cabc_setup_backlight;
+			panel->backlight.enable = cabc_enable_backlight;
+			panel->backlight.disable = cabc_disable_backlight;
+			panel->backlight.set = cabc_set_backlight;
+			panel->backlight.get = cabc_get_backlight;
+		} else {
+			panel->backlight.setup = bxt_setup_backlight;
+			panel->backlight.enable = bxt_enable_backlight;
+			panel->backlight.disable = bxt_disable_backlight;
+			panel->backlight.set = bxt_set_backlight;
+			panel->backlight.get = bxt_get_backlight;
+		}
 	} else if (HAS_PCH_LPT(dev) || HAS_PCH_SPT(dev)) {
 		panel->backlight.setup = lpt_setup_backlight;
 		panel->backlight.enable = lpt_enable_backlight;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control

[Jani Nikula]

On Mon, 16 Nov 2015, Deepak M <m.deepak@intel.com> wrote:
> In CABC (Content Adaptive Brightness Control) content grey level
> scale can be increased while simultaneously decreasing
> brightness of the backlight to achieve same perceived brightness.
>
> The CABC is not standardized and panel vendors are free to follow
> their implementation. The CABC implementaion here assumes that the
> panels use standard SW register for control.
>
> In this design there will be no PWM signal from the SoC and DCS
> commands are sent to enable and control the backlight brightness.

I'd like you to move the DSI DCS backlight stuff from intel_panel.c to a
new file, say, intel_dsi_dcs_backlight.c, similar to what I told Yetunde
to do with DP DPCD backlight [1].

[1] http://patchwork.freedesktop.org/patch/msgid/1447698547-9027-1-git-send-email-yetundex.adebisi@intel.com

> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> ---
> Addressed the review comments from Jani which were mentioned in
> the below patch
> http://lists.freedesktop.org/archives/intel-gfx/2015-September/075819.html
>
> Went with the name CABC because the commands which are used
> here are mainly for CABC and many of the panels have the same
> commands for the CABC operation. For the cases where PWM is
> directly from panel PWM, DCS commands may be different and
> may have to add new functions to support it.
>
>  drivers/gpu/drm/i915/intel_dsi.c   |  14 +++-
>  drivers/gpu/drm/i915/intel_dsi.h   |  24 ++++++
>  drivers/gpu/drm/i915/intel_panel.c | 145 +++++++++++++++++++++++++++++++++++--
>  3 files changed, 177 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 170ae6f..5d1ba35 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1175,8 +1175,20 @@ void intel_dsi_init(struct drm_device *dev)
>  		intel_dsi->ports = (1 << PORT_C);
>  	}
>  
> -	if (dev_priv->vbt.dsi.config->dual_link)
> +	if (dev_priv->vbt.dsi.config->dual_link) {
>  		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
> +		switch (dev_priv->vbt.dsi.config->dl_cabc_port) {
> +		case CABC_PORT_A:
> +			intel_dsi->bkl_dcs_ports = (1 << PORT_A);
> +		case CABC_PORT_C:
> +			intel_dsi->bkl_dcs_ports = (1 << PORT_C);
> +		case CABC_PORT_A_AND_C:
> +			intel_dsi->bkl_dcs_ports = intel_dsi->ports;
> +		default:
> +			DRM_ERROR("Unknown MIPI ports for sending DCS\n");

"break;" is useful in making the switch case do what you want.

> +		}
> +	} else
> +		intel_dsi->bkl_dcs_ports = intel_dsi->ports;
>  
>  	/* Create a DSI host (and a device) for each port. */
>  	for_each_dsi_port(port, intel_dsi->ports) {
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 4fde83b..4bcee40 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -34,6 +34,30 @@
>  #define DSI_DUAL_LINK_FRONT_BACK	1
>  #define DSI_DUAL_LINK_PIXEL_ALT		2
>  
> +#define CABC_OFF			(0 << 0)
> +#define CABC_USER_INTERFACE_IMAGE	(1 << 0)
> +#define CABC_STILL_PICTURE		(2 << 0)
> +#define CABC_VIDEO_MODE			(3 << 0)
> +
> +#define CABC_BACKLIGHT			(1 << 2)
> +#define CABC_DIMMING_DISPLAY		(1 << 3)
> +#define CABC_BCTRL			(1 << 5)
> +
> +#define CABC_PORT_A			0x00
> +#define CABC_PORT_C			0x01
> +#define CABC_PORT_A_AND_C		0x02
> +
> +#define CABC_MAX_VALUE			0xFF
> +
> +#define MIPI_DCS_CABC_LEVEL_RD		0x52
> +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_RD	0x5F
> +#define MIPI_DCS_CABC_CONTROL_RD	0x56
> +#define MIPI_DCS_CABC_CONTROL_BRIGHT_RD	0x54
> +#define MIPI_DCS_CABC_LEVEL_WR		0x51
> +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_WR	0x5E
> +#define MIPI_DCS_CABC_CONTROL_WR	0x55
> +#define MIPI_DCS_CABC_CONTROL_BRIGHT_WR	0x53
> +

Put these in the new .c file. The rest of the code shouldn't need to know.

>  struct intel_dsi_host;
>  
>  struct intel_dsi {
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index a24df35..085d9a6 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -34,6 +34,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/pwm.h>
>  #include "intel_drv.h"
> +#include "intel_dsi.h"
>  
>  #define CRC_PMIC_PWM_PERIOD_NS	21333
>  
> @@ -533,6 +534,30 @@ static u32 vlv_get_backlight(struct intel_connector *connector)
>  	return _vlv_get_backlight(dev, pipe);
>  }
>  
> +static u32 cabc_get_backlight(struct intel_connector *connector)
> +{
> +	struct intel_dsi *intel_dsi = NULL;
> +	struct intel_encoder *encoder = NULL;
> +	struct mipi_dsi_device *dsi_device;
> +	u8 data[2] = {0};
> +	enum port port;
> +
> +	encoder = connector->encoder;
> +	if (encoder->type == INTEL_OUTPUT_DSI)
> +		intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	else {
> +		DRM_ERROR("Use DSI encoder for CABC\n");
> +		return -EINVAL;
> +	}

You need to check this once when you setup the CABC backlight, and not
afterwards. In the function that sets up the CABC backlight hooks, not
the setup hook itself.

> +
> +	for_each_dsi_port(port, intel_dsi->bkl_dcs_ports) {
> +		dsi_device = intel_dsi->dsi_hosts[port]->device;
> +		mipi_dsi_dcs_read(dsi_device, MIPI_DCS_CABC_LEVEL_RD, data, 2);
> +	}
> +
> +	return data[1];
> +}
> +
>  static u32 bxt_get_backlight(struct intel_connector *connector)
>  {
>  	struct drm_device *dev = connector->base.dev;
> @@ -631,6 +656,30 @@ static void vlv_set_backlight(struct intel_connector *connector, u32 level)
>  	I915_WRITE(VLV_BLC_PWM_CTL(pipe), tmp | level);
>  }
>  
> +static void cabc_set_backlight(struct intel_connector *connector, u32 level)
> +{
> +	struct intel_dsi *intel_dsi = NULL;
> +	struct intel_encoder *encoder = NULL;
> +	struct mipi_dsi_device *dsi_device;
> +	u8 data[2] = {0};
> +	enum port port;
> +
> +	encoder = connector->encoder;
> +	if (encoder->type == INTEL_OUTPUT_DSI)
> +		intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	else {
> +		DRM_ERROR("Use DSI encoder for CABC\n");
> +		return;
> +	}

Same as above.

> +
> +	for_each_dsi_port(port, intel_dsi->bkl_dcs_ports) {
> +		dsi_device = intel_dsi->dsi_hosts[port]->device;
> +		data[1] = level;
> +		data[0] = MIPI_DCS_CABC_LEVEL_WR;
> +		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
> +	}
> +}
> +
>  static void bxt_set_backlight(struct intel_connector *connector, u32 level)
>  {
>  	struct drm_device *dev = connector->base.dev;
> @@ -798,6 +847,34 @@ static void vlv_disable_backlight(struct intel_connector *connector)
>  	I915_WRITE(VLV_BLC_PWM_CTL2(pipe), tmp & ~BLM_PWM_ENABLE);
>  }
>  
> +static void cabc_disable_backlight(struct intel_connector *connector)
> +{
> +	struct intel_dsi *intel_dsi = NULL;
> +	struct intel_encoder *encoder = NULL;
> +	struct mipi_dsi_device *dsi_device;
> +	enum port port;
> +	u8 data[2] = {0};
> +
> +	encoder = connector->encoder;
> +	if (encoder->type == INTEL_OUTPUT_DSI)
> +		intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	else {
> +		DRM_ERROR("Use DSI encoder for CABC\n");
> +		return;
> +	}

Same as above.

> +
> +	intel_panel_actually_set_backlight(connector, 0);
> +
> +	for_each_dsi_port(port, intel_dsi->bkl_dcs_ports) {
> +		dsi_device = intel_dsi->dsi_hosts[port]->device;
> +		data[1] = CABC_OFF;
> +		data[0] = MIPI_DCS_CABC_CONTROL_WR;
> +		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
> +		data[0] = MIPI_DCS_CABC_CONTROL_BRIGHT_WR;
> +		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
> +	}
> +}
> +
>  static void bxt_disable_backlight(struct intel_connector *connector)
>  {
>  	struct drm_device *dev = connector->base.dev;
> @@ -1133,6 +1210,36 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
>  	mutex_unlock(&dev_priv->backlight_lock);
>  }
>  
> +static void cabc_enable_backlight(struct intel_connector *connector)
> +{
> +	struct intel_dsi *intel_dsi = NULL;
> +	struct intel_encoder *encoder = NULL;
> +	struct intel_panel *panel = &connector->panel;
> +	struct mipi_dsi_device *dsi_device;
> +	enum port port;
> +	u8 data[2] = {0};
> +
> +	encoder = connector->encoder;
> +	if (encoder->type == INTEL_OUTPUT_DSI)
> +		intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	else {
> +		DRM_ERROR("Use DSI encoder for the CABC\n");
> +		return;
> +	}


Same as above.

> +
> +	for_each_dsi_port(port, intel_dsi->bkl_dcs_ports) {
> +		dsi_device = intel_dsi->dsi_hosts[port]->device;
> +		data[0] = MIPI_DCS_CABC_CONTROL_BRIGHT_WR;
> +		data[1] = CABC_BACKLIGHT | CABC_DIMMING_DISPLAY | CABC_BCTRL;
> +		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
> +		data[0] = MIPI_DCS_CABC_CONTROL_WR;
> +		data[1] = CABC_STILL_PICTURE;
> +		mipi_dsi_dcs_write_buffer(dsi_device, data, 2);
> +	}
> +
> +	intel_panel_actually_set_backlight(connector, panel->backlight.level);
> +}
> +
>  #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  static int intel_backlight_device_update_status(struct backlight_device *bd)
>  {
> @@ -1601,6 +1708,26 @@ static int vlv_setup_backlight(struct intel_connector *connector, enum pipe pipe
>  	return 0;
>  }
>  
> +static int cabc_setup_backlight(struct intel_connector *connector,
> +		enum pipe unused)
> +{
> +	struct drm_device *dev = connector->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_panel *panel = &connector->panel;
> +
> +	if (dev_priv->vbt.backlight.present)
> +		panel->backlight.present = true;
> +	else {
> +		DRM_ERROR("no backlight present per VBT\n");
> +		return 0;
> +	}

Same as above.

> +
> +	panel->backlight.max = CABC_MAX_VALUE;
> +	panel->backlight.level = CABC_MAX_VALUE;
> +
> +	return 0;
> +}
> +
>  static int
>  bxt_setup_backlight(struct intel_connector *connector, enum pipe unused)
>  {
> @@ -1745,11 +1872,19 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	if (IS_BROXTON(dev)) {
> -		panel->backlight.setup = bxt_setup_backlight;
> -		panel->backlight.enable = bxt_enable_backlight;
> -		panel->backlight.disable = bxt_disable_backlight;
> -		panel->backlight.set = bxt_set_backlight;
> -		panel->backlight.get = bxt_get_backlight;
> +		if (dev_priv->vbt.dsi.config->cabc_supported) {
> +			panel->backlight.setup = cabc_setup_backlight;
> +			panel->backlight.enable = cabc_enable_backlight;
> +			panel->backlight.disable = cabc_disable_backlight;
> +			panel->backlight.set = cabc_set_backlight;
> +			panel->backlight.get = cabc_get_backlight;
> +		} else {
> +			panel->backlight.setup = bxt_setup_backlight;
> +			panel->backlight.enable = bxt_enable_backlight;
> +			panel->backlight.disable = bxt_disable_backlight;
> +			panel->backlight.set = bxt_set_backlight;
> +			panel->backlight.get = bxt_get_backlight;
> +		}

Why is this Broxton specific? It's DSI specific, so please do this
similar to what I told Yetunde to do with DP AUX backlight.


>  	} else if (HAS_PCH_LPT(dev) || HAS_PCH_SPT(dev)) {
>  		panel->backlight.setup = lpt_setup_backlight;
>  		panel->backlight.enable = lpt_enable_backlight;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [MIPI CABC 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT

[Jani Nikula]

On Mon, 16 Nov 2015, Deepak M <m.deepak@intel.com> wrote:
> For dual link panel scenarios there are new fileds added in the
> VBT which indicate on which port the PWM cntrl and CABC ON/OFF
> commands needs to be sent.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
> Signed-off-by: Deepak M <m.deepak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 13 +++++++++++++
>  drivers/gpu/drm/i915/intel_bios.h |  5 ++++-
>  drivers/gpu/drm/i915/intel_dsi.h  |  2 ++
>  3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index ce82f9c..2ef8721 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -793,6 +793,19 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
>  		return;
>  	}
>  
> +	/*
> +	 * These below bits will inform us on which port the panel blk_cntrl and
> +	 * CABC ON/OFF commands needs to be sent in case of dual link panels
> +	 *	u16 dl_cabc_port:2;
> +	 *	u16 pwm_bkl_ctrl:2;
> +	 * But these are introduced from the VBT version 197 onwards, so making
> +	 * sure that these bits are zero in the pervious versions.
> +	 */
> +	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
> +		dev_priv->vbt.dsi.config->dl_cabc_port = 0;
> +		dev_priv->vbt.dsi.config->pwm_bkl_ctrl = 0;
> +	}
> +

dev_priv->vbt is zero by default, so none of the above is needed.

>  	/* We have mandatory mipi config blocks. Initialize as generic panel */
>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
>  
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 7ec8c9a..9283969 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -832,7 +832,10 @@ struct mipi_config {
>  	u16 dual_link:2;
>  	u16 lane_cnt:2;
>  	u16 pixel_overlap:3;
> -	u16 rsvd3:9;
> +	u16 rgb_flip:1;
> +	u16 dl_cabc_port:2;
> +	u16 pwm_bkl_ctrl:2;
> +	u16 rsvd3:4;
>  
>  	u16 rsvd4;
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index e6cb252..4fde83b 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -74,6 +74,8 @@ struct intel_dsi {
>  
>  	u8 escape_clk_div;
>  	u8 dual_link;
> +	u8 bkl_dcs_ports;
> +	u8 pwm_blk_ctrl;

This is a much more natural place to explain what these things do.

>  	u8 pixel_overlap;
>  	u32 port_bits;
>  	u32 bw_timer;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [MIPI CABC 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT

[Jani Nikula]

On Wed, 25 Nov 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 16 Nov 2015, Deepak M <m.deepak@intel.com> wrote:
>> For dual link panel scenarios there are new fileds added in the
>> VBT which indicate on which port the PWM cntrl and CABC ON/OFF
>> commands needs to be sent.
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>
>> Signed-off-by: Deepak M <m.deepak@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_bios.c | 13 +++++++++++++
>>  drivers/gpu/drm/i915/intel_bios.h |  5 ++++-
>>  drivers/gpu/drm/i915/intel_dsi.h  |  2 ++
>>  3 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index ce82f9c..2ef8721 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -793,6 +793,19 @@ parse_mipi(struct drm_i915_private *dev_priv, const struct bdb_header *bdb)
>>  		return;
>>  	}
>>  
>> +	/*
>> +	 * These below bits will inform us on which port the panel blk_cntrl and
>> +	 * CABC ON/OFF commands needs to be sent in case of dual link panels
>> +	 *	u16 dl_cabc_port:2;
>> +	 *	u16 pwm_bkl_ctrl:2;
>> +	 * But these are introduced from the VBT version 197 onwards, so making
>> +	 * sure that these bits are zero in the pervious versions.
>> +	 */
>> +	if (dev_priv->vbt.dsi.config->dual_link && bdb->version < 197) {
>> +		dev_priv->vbt.dsi.config->dl_cabc_port = 0;
>> +		dev_priv->vbt.dsi.config->pwm_bkl_ctrl = 0;
>> +	}
>> +
>
> dev_priv->vbt is zero by default, so none of the above is needed.

Ugh, I didn't realize we just copy the dsi.config verbatim from
VBT. Maybe we do want this after all.


>
>>  	/* We have mandatory mipi config blocks. Initialize as generic panel */
>>  	dev_priv->vbt.dsi.panel_id = MIPI_DSI_GENERIC_PANEL_ID;
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
>> index 7ec8c9a..9283969 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.h
>> +++ b/drivers/gpu/drm/i915/intel_bios.h
>> @@ -832,7 +832,10 @@ struct mipi_config {
>>  	u16 dual_link:2;
>>  	u16 lane_cnt:2;
>>  	u16 pixel_overlap:3;
>> -	u16 rsvd3:9;
>> +	u16 rgb_flip:1;
>> +	u16 dl_cabc_port:2;
>> +	u16 pwm_bkl_ctrl:2;
>> +	u16 rsvd3:4;
>>  
>>  	u16 rsvd4;
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
>> index e6cb252..4fde83b 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.h
>> +++ b/drivers/gpu/drm/i915/intel_dsi.h
>> @@ -74,6 +74,8 @@ struct intel_dsi {
>>  
>>  	u8 escape_clk_div;
>>  	u8 dual_link;
>> +	u8 bkl_dcs_ports;
>> +	u8 pwm_blk_ctrl;
>
> This is a much more natural place to explain what these things do.
>
>>  	u8 pixel_overlap;
>>  	u32 port_bits;
>>  	u32 bw_timer;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[MIPI CABC 0/2] CABC patch list

[Deepak M]

CABC stands for the Content Adaptive Backlight Control.
In the normal display the backlight which we see is due to the
backlight which is being modulated by the filter, which is inturn
dependent on the image. In brief the CABC does the histogram
analysis of the image and then controls the filter and backlight.
For example in CABC to display the dark image the backlight is dimmed
and then controlls the filter to allow more light, because of
which is power consuption will be reduced.

Below are the initial set of patches which supports the CABC.
A field exists in the mipi configuration of the VBT which
when enabled indicates the CABC is supported. Depending on
this field the backlight control function pointer are
initialized in the intel_panel.c file.

In case of dual link panels depending on the panel
the DCS commands have to be send to either PORT A,
PORT C or both PORT A and PORT C. Again a field is
added in the VBT to get this data from the version 197 onwards.
One of the below patches parses these fields from the
VBT.

Addressed the review comments of Jani, which were mentioned in
the below link
https://lists.freedesktop.org/archives/intel-gfx/2015-November/081233.html

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Yetunde Adebisi <yetundex.adebisi@intel.com>

Deepak M (2):
  drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT
  drm/i915: CABC support for backlight control

 drivers/gpu/drm/i915/Makefile         |   1 +
 drivers/gpu/drm/i915/i915_drv.h       |   2 +-
 drivers/gpu/drm/i915/intel_bios.c     |  10 ++
 drivers/gpu/drm/i915/intel_bios.h     |   5 +-
 drivers/gpu/drm/i915/intel_dsi.c      |  19 +++-
 drivers/gpu/drm/i915/intel_dsi.h      |  13 +++
 drivers/gpu/drm/i915/intel_dsi_cabc.c | 179 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_panel.c    |   4 +
 8 files changed, 229 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx