* [MIPI CABC 0/2] CABC patch list
@ 2016-03-22 5:41 Deepak M
2016-03-22 5:41 ` [MIPI CABC 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT Deepak M
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Deepak M @ 2016-03-22 5:41 UTC (permalink / raw)
To: intel-gfx; +Cc: Deepak M, Jani Nikula, Yetunde Adebisi, Daniel Vetter
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
^ permalink raw reply [flat|nested] 10+ messages in thread* [MIPI CABC 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT 2016-03-22 5:41 [MIPI CABC 0/2] CABC patch list Deepak M @ 2016-03-22 5:41 ` Deepak M 2016-03-22 5:41 ` [MIPI CABC 2/2] drm/i915: CABC support for backlight control Deepak M 2016-03-22 10:04 ` ✗ Fi.CI.BAT: failure for CABC patch list (rev4) Patchwork 2 siblings, 0 replies; 10+ messages in thread From: Deepak M @ 2016-03-22 5:41 UTC (permalink / raw) To: intel-gfx; +Cc: Deepak M, Jani Nikula, Yetunde Adebisi, Daniel Vetter 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. v2: Moving the comment to intel_dsi.h(Jani) 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 | 10 ++++++++++ drivers/gpu/drm/i915/intel_bios.h | 5 ++++- drivers/gpu/drm/i915/intel_dsi.h | 9 +++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 083003b..587c06f 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -749,6 +749,16 @@ parse_mipi_config(struct drm_i915_private *dev_priv, return; } + /* + * These fileds are introduced from the VBT version 197 onwards, + * so making sure that these bits are set 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 ab0ea31..7a89f79 100644 --- a/drivers/gpu/drm/i915/intel_bios.h +++ b/drivers/gpu/drm/i915/intel_bios.h @@ -113,7 +113,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 e582ef8..0e758f1 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -78,6 +78,15 @@ struct intel_dsi { u8 escape_clk_div; u8 dual_link; + + /* + * Below field 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 + */ + 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 https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [MIPI CABC 2/2] drm/i915: CABC support for backlight control 2016-03-22 5:41 [MIPI CABC 0/2] CABC patch list Deepak M 2016-03-22 5:41 ` [MIPI CABC 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT Deepak M @ 2016-03-22 5:41 ` Deepak M 2016-03-22 13:48 ` Jani Nikula 2016-03-22 10:04 ` ✗ Fi.CI.BAT: failure for CABC patch list (rev4) Patchwork 2 siblings, 1 reply; 10+ messages in thread From: Deepak M @ 2016-03-22 5:41 UTC (permalink / raw) To: intel-gfx; +Cc: Deepak M, Jani Nikula, Yetunde Adebisi, Daniel Vetter 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. v2: Moving the CABC bkl functions to new file.(Jani) v3: Rebase v4: Rebase 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/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_dsi.c | 19 +++- drivers/gpu/drm/i915/intel_dsi.h | 4 + drivers/gpu/drm/i915/intel_dsi_cabc.c | 179 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_panel.c | 4 + 6 files changed, 206 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 7ffb51b..065c410 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -83,6 +83,7 @@ i915-y += dvo_ch7017.o \ intel_dp_mst.o \ intel_dp.o \ intel_dsi.o \ + intel_dsi_cabc.o \ intel_dsi_panel_vbt.o \ intel_dsi_pll.o \ intel_dvo.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 050d860..d196404 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3489,7 +3489,7 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value, enum intel_sbi_destination destination); u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg); void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); - +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector *intel_connector); int intel_gpu_freq(struct drm_i915_private *dev_priv, int val); int intel_freq_opcode(struct drm_i915_private *dev_priv, int val); diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 456676c..7aa707f 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -1209,10 +1209,25 @@ void intel_dsi_init(struct drm_device *dev) else intel_encoder->crtc_mask = BIT(PIPE_B); - if (dev_priv->vbt.dsi.config->dual_link) + if (dev_priv->vbt.dsi.config->dual_link) { intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C); - else + switch (dev_priv->vbt.dsi.config->dl_cabc_port) { + case CABC_PORT_A: + intel_dsi->bkl_dcs_ports = BIT(PORT_A); + break; + case CABC_PORT_C: + intel_dsi->bkl_dcs_ports = BIT(PORT_C); + break; + case CABC_PORT_A_AND_C: + intel_dsi->bkl_dcs_ports = BIT(PORT_A) | BIT(PORT_C); + break; + default: + DRM_ERROR("Unknown MIPI ports for sending DCS\n"); + } + } else { intel_dsi->ports = BIT(port); + intel_dsi->bkl_dcs_ports = BIT(port); + } /* 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 0e758f1..5c07d59 100644 --- a/drivers/gpu/drm/i915/intel_dsi.h +++ b/drivers/gpu/drm/i915/intel_dsi.h @@ -34,6 +34,10 @@ #define DSI_DUAL_LINK_FRONT_BACK 1 #define DSI_DUAL_LINK_PIXEL_ALT 2 +#define CABC_PORT_A 0x00 +#define CABC_PORT_C 0x01 +#define CABC_PORT_A_AND_C 0x02 + struct intel_dsi_host; struct intel_dsi { diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c b/drivers/gpu/drm/i915/intel_dsi_cabc.c new file mode 100644 index 0000000..d14a669 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c @@ -0,0 +1,179 @@ +/* + * Copyright © 2006-2010 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Author: Deepak M <m.deepak at intel.com> + */ + +#include "intel_drv.h" +#include "intel_dsi.h" +#include "i915_drv.h" +#include <drm/drm_mipi_dsi.h> + +#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_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 + +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; + intel_dsi = enc_to_intel_dsi(&encoder->base); + + 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 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; + intel_dsi = enc_to_intel_dsi(&encoder->base); + + 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 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; + intel_dsi = enc_to_intel_dsi(&encoder->base); + + cabc_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 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; + intel_dsi = enc_to_intel_dsi(&encoder->base); + + 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); + } + + cabc_set_backlight(connector, panel->backlight.level); +} + +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; +} + +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector *intel_connector) +{ + struct drm_device *dev = intel_connector->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_encoder *encoder = intel_connector->encoder; + struct intel_panel *panel = &intel_connector->panel; + + if (!dev_priv->vbt.dsi.config->cabc_supported) + return -EINVAL; + + if (encoder->type != INTEL_OUTPUT_DSI) { + DRM_ERROR("Use DSI encoder for CABC\n"); + return -EINVAL; + } + + 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; + + return 0; +} diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 8c8996f..0f9bf80 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1718,6 +1718,10 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) container_of(panel, struct intel_connector, panel); struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI && + intel_dsi_cabc_init_backlight_funcs(connector) == 0) + return; + if (IS_BROXTON(dev_priv)) { panel->backlight.setup = bxt_setup_backlight; panel->backlight.enable = bxt_enable_backlight; -- 1.9.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control 2016-03-22 5:41 ` [MIPI CABC 2/2] drm/i915: CABC support for backlight control Deepak M @ 2016-03-22 13:48 ` Jani Nikula 2016-03-23 10:49 ` Deepak, M 0 siblings, 1 reply; 10+ messages in thread From: Jani Nikula @ 2016-03-22 13:48 UTC (permalink / raw) To: intel-gfx; +Cc: Deepak M, Daniel Vetter, Yetunde Adebisi On Tue, 22 Mar 2016, 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. > > v2: Moving the CABC bkl functions to new file.(Jani) > > v3: Rebase > > v4: Rebase > > 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/Makefile | 1 + > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_dsi.c | 19 +++- > drivers/gpu/drm/i915/intel_dsi.h | 4 + > drivers/gpu/drm/i915/intel_dsi_cabc.c | 179 ++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_panel.c | 4 + > 6 files changed, 206 insertions(+), 3 deletions(-) > create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 7ffb51b..065c410 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -83,6 +83,7 @@ i915-y += dvo_ch7017.o \ > intel_dp_mst.o \ > intel_dp.o \ > intel_dsi.o \ > + intel_dsi_cabc.o \ > intel_dsi_panel_vbt.o \ > intel_dsi_pll.o \ > intel_dvo.o \ > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 050d860..d196404 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3489,7 +3489,7 @@ void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value, > enum intel_sbi_destination destination); > u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg); > void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); > - > +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector *intel_connector); This probably fits better in intel_drv.h under a /* intel_dsi_cabc.c */ comment, see the file for examples. > int intel_gpu_freq(struct drm_i915_private *dev_priv, int val); > int intel_freq_opcode(struct drm_i915_private *dev_priv, int val); > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 456676c..7aa707f 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -1209,10 +1209,25 @@ void intel_dsi_init(struct drm_device *dev) > else > intel_encoder->crtc_mask = BIT(PIPE_B); > > - if (dev_priv->vbt.dsi.config->dual_link) > + if (dev_priv->vbt.dsi.config->dual_link) { > intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C); > - else > + switch (dev_priv->vbt.dsi.config->dl_cabc_port) { > + case CABC_PORT_A: > + intel_dsi->bkl_dcs_ports = BIT(PORT_A); > + break; > + case CABC_PORT_C: > + intel_dsi->bkl_dcs_ports = BIT(PORT_C); > + break; > + case CABC_PORT_A_AND_C: > + intel_dsi->bkl_dcs_ports = BIT(PORT_A) | BIT(PORT_C); > + break; > + default: > + DRM_ERROR("Unknown MIPI ports for sending DCS\n"); > + } > + } else { > intel_dsi->ports = BIT(port); > + intel_dsi->bkl_dcs_ports = BIT(port); > + } > > /* 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 0e758f1..5c07d59 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.h > +++ b/drivers/gpu/drm/i915/intel_dsi.h > @@ -34,6 +34,10 @@ > #define DSI_DUAL_LINK_FRONT_BACK 1 > #define DSI_DUAL_LINK_PIXEL_ALT 2 > > +#define CABC_PORT_A 0x00 > +#define CABC_PORT_C 0x01 > +#define CABC_PORT_A_AND_C 0x02 > + > struct intel_dsi_host; > > struct intel_dsi { > diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c b/drivers/gpu/drm/i915/intel_dsi_cabc.c > new file mode 100644 > index 0000000..d14a669 > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c > @@ -0,0 +1,179 @@ > +/* > + * Copyright © 2006-2010 Intel Corporation 2016 > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Author: Deepak M <m.deepak at intel.com> > + */ > + > +#include "intel_drv.h" > +#include "intel_dsi.h" > +#include "i915_drv.h" > +#include <drm/drm_mipi_dsi.h> > + > +#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_MAX_VALUE 0xFF > + The defines below should be added to the relevant enum in include/video/mipi_display.h using the MIPI DCS specification names. I'll copy them below. Please create a separate prep patch and send it to dri-devel. > +#define MIPI_DCS_CABC_LEVEL_RD 0x52 MIPI_DCS_GET_DISPLAY_BRIGHTNESS > +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_RD 0x5F MIPI_DCS_GET_CABC_MIN_BRIGHTNESS > +#define MIPI_DCS_CABC_CONTROL_RD 0x56 MIPI_DCS_GET_POWER_SAVE > +#define MIPI_DCS_CABC_CONTROL_BRIGHT_RD 0x54 MIPI_DCS_GET_CONTROL_DISPLAY > +#define MIPI_DCS_CABC_LEVEL_WR 0x51 MIPI_DCS_SET_DISPLAY_BRIGHTNESS > +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_WR 0x5E MIPI_DCS_SET_CABC_MIN_BRIGHTNESS > +#define MIPI_DCS_CABC_CONTROL_WR 0x55 MIPI_DCS_WRITE_POWER_SAVE > +#define MIPI_DCS_CABC_CONTROL_BRIGHT_WR 0x53 MIPI_DCS_WRITE_CONTROL_DISPLAY > + > +static u32 cabc_get_backlight(struct intel_connector *connector) > +{ struct intel_encoder *encoder = connector->encoder; struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); Same for all functions below. > + 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; > + intel_dsi = enc_to_intel_dsi(&encoder->base); > + > + 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]; Hmm, it's up to the manufacturer whether the this is 8 or 16 bits. I guess you're just assuming 8 bits? I wonder if this should be more generic wrt 8 vs. 16 bits. > +} > + > +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; > + intel_dsi = enc_to_intel_dsi(&encoder->base); > + > + 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); Please use mipi_dsi_dcs_write(). Same for all functions below. > + } > +} > + > +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; > + intel_dsi = enc_to_intel_dsi(&encoder->base); > + > + cabc_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 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; > + intel_dsi = enc_to_intel_dsi(&encoder->base); > + > + 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); > + } > + > + cabc_set_backlight(connector, panel->backlight.level); > +} > + > +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; > + } None of the above is needed. dev_priv->vbt.backlight.present is checked higher level in intel_panel_setup_backlight(), and panel->backlight.present = true is set there as well if this hook returns 0. > + > + panel->backlight.max = CABC_MAX_VALUE; > + panel->backlight.level = CABC_MAX_VALUE; > + > + return 0; > +} > + > +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector *intel_connector) > +{ > + struct drm_device *dev = intel_connector->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_encoder *encoder = intel_connector->encoder; > + struct intel_panel *panel = &intel_connector->panel; > + > + if (!dev_priv->vbt.dsi.config->cabc_supported) > + return -EINVAL; -ENODEV seems more appropriate. > + > + if (encoder->type != INTEL_OUTPUT_DSI) { > + DRM_ERROR("Use DSI encoder for CABC\n"); > + return -EINVAL; > + } if (WARN_ON(encoder->type != INTEL_OUTPUT_DSI)) return -EINVAL; > + > + 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; > + > + return 0; > +} > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 8c8996f..0f9bf80 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1718,6 +1718,10 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) > container_of(panel, struct intel_connector, panel); > struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > + if (connector->base.connector_type == DRM_MODE_CONNECTOR_DSI && > + intel_dsi_cabc_init_backlight_funcs(connector) == 0) Indentation is not quite right. > + return; > + > if (IS_BROXTON(dev_priv)) { > panel->backlight.setup = bxt_setup_backlight; > panel->backlight.enable = bxt_enable_backlight; -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control 2016-03-22 13:48 ` Jani Nikula @ 2016-03-23 10:49 ` Deepak, M 2016-03-23 11:50 ` Jani Nikula 0 siblings, 1 reply; 10+ messages in thread From: Deepak, M @ 2016-03-23 10:49 UTC (permalink / raw) To: Nikula, Jani, intel-gfx; +Cc: Adebisi, YetundeX, Vetter, Daniel > -----Original Message----- > From: Nikula, Jani > Sent: Tuesday, March 22, 2016 7:19 PM > To: Deepak, M <m.deepak@intel.com>; intel-gfx@lists.freedesktop.org > Cc: Deepak, M <m.deepak@intel.com>; Vetter, Daniel > <daniel.vetter@intel.com>; Adebisi, YetundeX > <yetundex.adebisi@intel.com> > Subject: Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control > > On Tue, 22 Mar 2016, 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. > > > > v2: Moving the CABC bkl functions to new file.(Jani) > > > > v3: Rebase > > > > v4: Rebase > > > > 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/Makefile | 1 + > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_dsi.c | 19 +++- > > drivers/gpu/drm/i915/intel_dsi.h | 4 + > > drivers/gpu/drm/i915/intel_dsi_cabc.c | 179 > ++++++++++++++++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_panel.c | 4 + > > 6 files changed, 206 insertions(+), 3 deletions(-) create mode > > 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c > > > > diff --git a/drivers/gpu/drm/i915/Makefile > > b/drivers/gpu/drm/i915/Makefile index 7ffb51b..065c410 100644 > > --- a/drivers/gpu/drm/i915/Makefile > > +++ b/drivers/gpu/drm/i915/Makefile > > @@ -83,6 +83,7 @@ i915-y += dvo_ch7017.o \ > > intel_dp_mst.o \ > > intel_dp.o \ > > intel_dsi.o \ > > + intel_dsi_cabc.o \ > > intel_dsi_panel_vbt.o \ > > intel_dsi_pll.o \ > > intel_dvo.o \ > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h index 050d860..d196404 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -3489,7 +3489,7 @@ void intel_sbi_write(struct drm_i915_private > *dev_priv, u16 reg, u32 value, > > enum intel_sbi_destination destination); > > u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg); > > void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 > > val); > > - > > +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector > > +*intel_connector); > > This probably fits better in intel_drv.h under a /* intel_dsi_cabc.c */ > comment, see the file for examples. > > > int intel_gpu_freq(struct drm_i915_private *dev_priv, int val); int > > intel_freq_opcode(struct drm_i915_private *dev_priv, int val); > > > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c > > b/drivers/gpu/drm/i915/intel_dsi.c > > index 456676c..7aa707f 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -1209,10 +1209,25 @@ void intel_dsi_init(struct drm_device *dev) > > else > > intel_encoder->crtc_mask = BIT(PIPE_B); > > > > - if (dev_priv->vbt.dsi.config->dual_link) > > + if (dev_priv->vbt.dsi.config->dual_link) { > > intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C); > > - else > > + switch (dev_priv->vbt.dsi.config->dl_cabc_port) { > > + case CABC_PORT_A: > > + intel_dsi->bkl_dcs_ports = BIT(PORT_A); > > + break; > > + case CABC_PORT_C: > > + intel_dsi->bkl_dcs_ports = BIT(PORT_C); > > + break; > > + case CABC_PORT_A_AND_C: > > + intel_dsi->bkl_dcs_ports = BIT(PORT_A) | > BIT(PORT_C); > > + break; > > + default: > > + DRM_ERROR("Unknown MIPI ports for sending > DCS\n"); > > + } > > + } else { > > intel_dsi->ports = BIT(port); > > + intel_dsi->bkl_dcs_ports = BIT(port); > > + } > > > > /* 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 0e758f1..5c07d59 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.h > > +++ b/drivers/gpu/drm/i915/intel_dsi.h > > @@ -34,6 +34,10 @@ > > #define DSI_DUAL_LINK_FRONT_BACK 1 > > #define DSI_DUAL_LINK_PIXEL_ALT 2 > > > > +#define CABC_PORT_A 0x00 > > +#define CABC_PORT_C 0x01 > > +#define CABC_PORT_A_AND_C 0x02 > > + > > struct intel_dsi_host; > > > > struct intel_dsi { > > diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c > > b/drivers/gpu/drm/i915/intel_dsi_cabc.c > > new file mode 100644 > > index 0000000..d14a669 > > --- /dev/null > > +++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c > > @@ -0,0 +1,179 @@ > > +/* > > + * Copyright © 2006-2010 Intel Corporation > > 2016 > > > + * > > + * Permission is hereby granted, free of charge, to any person > > + obtaining a > > + * copy of this software and associated documentation files (the > > + "Software"), > > + * to deal in the Software without restriction, including without > > + limitation > > + * the rights to use, copy, modify, merge, publish, distribute, > > + sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom > > + the > > + * Software is furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice (including > > + the next > > + * paragraph) shall be included in all copies or substantial portions > > + of the > > + * Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY > KIND, > > + EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > + MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN > NO EVENT > > + SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > DAMAGES > > + OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > OTHERWISE, > > + ARISING > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE > OR > > + OTHER > > + * DEALINGS IN THE SOFTWARE. > > + * > > + * Author: Deepak M <m.deepak at intel.com> */ > > + > > +#include "intel_drv.h" > > +#include "intel_dsi.h" > > +#include "i915_drv.h" > > +#include <drm/drm_mipi_dsi.h> > > + > > +#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_MAX_VALUE 0xFF > > + > > The defines below should be added to the relevant enum in > include/video/mipi_display.h using the MIPI DCS specification names. I'll > copy them below. Please create a separate prep patch and send it to dri- > devel. > > > +#define MIPI_DCS_CABC_LEVEL_RD 0x52 > > MIPI_DCS_GET_DISPLAY_BRIGHTNESS > > > +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_RD 0x5F > > MIPI_DCS_GET_CABC_MIN_BRIGHTNESS > > > +#define MIPI_DCS_CABC_CONTROL_RD 0x56 > > MIPI_DCS_GET_POWER_SAVE > > > +#define MIPI_DCS_CABC_CONTROL_BRIGHT_RD 0x54 > > MIPI_DCS_GET_CONTROL_DISPLAY > > > +#define MIPI_DCS_CABC_LEVEL_WR 0x51 > > MIPI_DCS_SET_DISPLAY_BRIGHTNESS > > > +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_WR 0x5E > > MIPI_DCS_SET_CABC_MIN_BRIGHTNESS > > > +#define MIPI_DCS_CABC_CONTROL_WR 0x55 > > MIPI_DCS_WRITE_POWER_SAVE > > > +#define MIPI_DCS_CABC_CONTROL_BRIGHT_WR 0x53 > > MIPI_DCS_WRITE_CONTROL_DISPLAY > > > + > > +static u32 cabc_get_backlight(struct intel_connector *connector) { > struct intel_encoder *encoder = connector->encoder; > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > > Same for all functions below. > > > + 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; > > + intel_dsi = enc_to_intel_dsi(&encoder->base); > > + > > + 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]; > > Hmm, it's up to the manufacturer whether the this is 8 or 16 bits. I guess > you're just assuming 8 bits? I wonder if this should be more generic wrt 8 vs. > 16 bits. > [Deepak, M] Need to think on how to make it generic wrt 8 vs 16 bits, problem here is to somehow we have to identify the no of parameters or we have to pass this info in VBT... > > +} > > + > > +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; > > + intel_dsi = enc_to_intel_dsi(&encoder->base); > > + > > + 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); > > Please use mipi_dsi_dcs_write(). Same for all functions below. > > > + } > > +} > > + > > +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; > > + intel_dsi = enc_to_intel_dsi(&encoder->base); > > + > > + cabc_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 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; > > + intel_dsi = enc_to_intel_dsi(&encoder->base); > > + > > + 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); > > + } > > + > > + cabc_set_backlight(connector, panel->backlight.level); } > > + > > +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; > > + } > > None of the above is needed. > > dev_priv->vbt.backlight.present is checked higher level in > intel_panel_setup_backlight(), and panel->backlight.present = true is set > there as well if this hook returns 0. > > > + > > + panel->backlight.max = CABC_MAX_VALUE; > > + panel->backlight.level = CABC_MAX_VALUE; > > + > > + return 0; > > +} > > + > > +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector > > +*intel_connector) { > > + struct drm_device *dev = intel_connector->base.dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + struct intel_encoder *encoder = intel_connector->encoder; > > + struct intel_panel *panel = &intel_connector->panel; > > + > > + if (!dev_priv->vbt.dsi.config->cabc_supported) > > + return -EINVAL; > > -ENODEV seems more appropriate. > > > + > > + if (encoder->type != INTEL_OUTPUT_DSI) { > > + DRM_ERROR("Use DSI encoder for CABC\n"); > > + return -EINVAL; > > + } > > if (WARN_ON(encoder->type != INTEL_OUTPUT_DSI)) > return -EINVAL; > > > + > > + 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; > > + > > + return 0; > > +} > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > b/drivers/gpu/drm/i915/intel_panel.c > > index 8c8996f..0f9bf80 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -1718,6 +1718,10 @@ intel_panel_init_backlight_funcs(struct > intel_panel *panel) > > container_of(panel, struct intel_connector, panel); > > struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > > > + if (connector->base.connector_type == > DRM_MODE_CONNECTOR_DSI && > > + intel_dsi_cabc_init_backlight_funcs(connector) == 0) > > Indentation is not quite right. > > > + return; > > + > > if (IS_BROXTON(dev_priv)) { > > panel->backlight.setup = bxt_setup_backlight; > > panel->backlight.enable = bxt_enable_backlight; > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control 2016-03-23 10:49 ` Deepak, M @ 2016-03-23 11:50 ` Jani Nikula 2016-03-23 11:51 ` Jani Nikula 0 siblings, 1 reply; 10+ messages in thread From: Jani Nikula @ 2016-03-23 11:50 UTC (permalink / raw) To: Deepak, M, intel-gfx; +Cc: Adebisi, YetundeX, Vetter, Daniel On Wed, 23 Mar 2016, "Deepak, M" <m.deepak@intel.com> wrote: >> -----Original Message----- >> From: Nikula, Jani >> Sent: Tuesday, March 22, 2016 7:19 PM >> To: Deepak, M <m.deepak@intel.com>; intel-gfx@lists.freedesktop.org >> Cc: Deepak, M <m.deepak@intel.com>; Vetter, Daniel >> <daniel.vetter@intel.com>; Adebisi, YetundeX >> <yetundex.adebisi@intel.com> >> Subject: Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control >> >> On Tue, 22 Mar 2016, 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. >> > >> > v2: Moving the CABC bkl functions to new file.(Jani) >> > >> > v3: Rebase >> > >> > v4: Rebase >> > >> > 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/Makefile | 1 + >> > drivers/gpu/drm/i915/i915_drv.h | 2 +- >> > drivers/gpu/drm/i915/intel_dsi.c | 19 +++- >> > drivers/gpu/drm/i915/intel_dsi.h | 4 + >> > drivers/gpu/drm/i915/intel_dsi_cabc.c | 179 >> ++++++++++++++++++++++++++++++++++ >> > drivers/gpu/drm/i915/intel_panel.c | 4 + >> > 6 files changed, 206 insertions(+), 3 deletions(-) create mode >> > 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c >> > >> > diff --git a/drivers/gpu/drm/i915/Makefile >> > b/drivers/gpu/drm/i915/Makefile index 7ffb51b..065c410 100644 >> > --- a/drivers/gpu/drm/i915/Makefile >> > +++ b/drivers/gpu/drm/i915/Makefile >> > @@ -83,6 +83,7 @@ i915-y += dvo_ch7017.o \ >> > intel_dp_mst.o \ >> > intel_dp.o \ >> > intel_dsi.o \ >> > + intel_dsi_cabc.o \ >> > intel_dsi_panel_vbt.o \ >> > intel_dsi_pll.o \ >> > intel_dvo.o \ >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h >> > b/drivers/gpu/drm/i915/i915_drv.h index 050d860..d196404 100644 >> > --- a/drivers/gpu/drm/i915/i915_drv.h >> > +++ b/drivers/gpu/drm/i915/i915_drv.h >> > @@ -3489,7 +3489,7 @@ void intel_sbi_write(struct drm_i915_private >> *dev_priv, u16 reg, u32 value, >> > enum intel_sbi_destination destination); >> > u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg); >> > void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 >> > val); >> > - >> > +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector >> > +*intel_connector); >> >> This probably fits better in intel_drv.h under a /* intel_dsi_cabc.c */ >> comment, see the file for examples. >> >> > int intel_gpu_freq(struct drm_i915_private *dev_priv, int val); int >> > intel_freq_opcode(struct drm_i915_private *dev_priv, int val); >> > >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c >> > b/drivers/gpu/drm/i915/intel_dsi.c >> > index 456676c..7aa707f 100644 >> > --- a/drivers/gpu/drm/i915/intel_dsi.c >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c >> > @@ -1209,10 +1209,25 @@ void intel_dsi_init(struct drm_device *dev) >> > else >> > intel_encoder->crtc_mask = BIT(PIPE_B); >> > >> > - if (dev_priv->vbt.dsi.config->dual_link) >> > + if (dev_priv->vbt.dsi.config->dual_link) { >> > intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C); >> > - else >> > + switch (dev_priv->vbt.dsi.config->dl_cabc_port) { >> > + case CABC_PORT_A: >> > + intel_dsi->bkl_dcs_ports = BIT(PORT_A); >> > + break; >> > + case CABC_PORT_C: >> > + intel_dsi->bkl_dcs_ports = BIT(PORT_C); >> > + break; >> > + case CABC_PORT_A_AND_C: >> > + intel_dsi->bkl_dcs_ports = BIT(PORT_A) | >> BIT(PORT_C); >> > + break; >> > + default: >> > + DRM_ERROR("Unknown MIPI ports for sending >> DCS\n"); >> > + } >> > + } else { >> > intel_dsi->ports = BIT(port); >> > + intel_dsi->bkl_dcs_ports = BIT(port); >> > + } >> > >> > /* 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 0e758f1..5c07d59 100644 >> > --- a/drivers/gpu/drm/i915/intel_dsi.h >> > +++ b/drivers/gpu/drm/i915/intel_dsi.h >> > @@ -34,6 +34,10 @@ >> > #define DSI_DUAL_LINK_FRONT_BACK 1 >> > #define DSI_DUAL_LINK_PIXEL_ALT 2 >> > >> > +#define CABC_PORT_A 0x00 >> > +#define CABC_PORT_C 0x01 >> > +#define CABC_PORT_A_AND_C 0x02 >> > + >> > struct intel_dsi_host; >> > >> > struct intel_dsi { >> > diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c >> > b/drivers/gpu/drm/i915/intel_dsi_cabc.c >> > new file mode 100644 >> > index 0000000..d14a669 >> > --- /dev/null >> > +++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c >> > @@ -0,0 +1,179 @@ >> > +/* >> > + * Copyright © 2006-2010 Intel Corporation >> >> 2016 >> >> > + * >> > + * Permission is hereby granted, free of charge, to any person >> > + obtaining a >> > + * copy of this software and associated documentation files (the >> > + "Software"), >> > + * to deal in the Software without restriction, including without >> > + limitation >> > + * the rights to use, copy, modify, merge, publish, distribute, >> > + sublicense, >> > + * and/or sell copies of the Software, and to permit persons to whom >> > + the >> > + * Software is furnished to do so, subject to the following conditions: >> > + * >> > + * The above copyright notice and this permission notice (including >> > + the next >> > + * paragraph) shall be included in all copies or substantial portions >> > + of the >> > + * Software. >> > + * >> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY >> KIND, >> > + EXPRESS OR >> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> > + MERCHANTABILITY, >> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN >> NO EVENT >> > + SHALL >> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, >> DAMAGES >> > + OR OTHER >> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >> OTHERWISE, >> > + ARISING >> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE >> OR >> > + OTHER >> > + * DEALINGS IN THE SOFTWARE. >> > + * >> > + * Author: Deepak M <m.deepak at intel.com> */ >> > + >> > +#include "intel_drv.h" >> > +#include "intel_dsi.h" >> > +#include "i915_drv.h" >> > +#include <drm/drm_mipi_dsi.h> >> > + >> > +#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_MAX_VALUE 0xFF >> > + >> >> The defines below should be added to the relevant enum in >> include/video/mipi_display.h using the MIPI DCS specification names. I'll >> copy them below. Please create a separate prep patch and send it to dri- >> devel. >> >> > +#define MIPI_DCS_CABC_LEVEL_RD 0x52 >> >> MIPI_DCS_GET_DISPLAY_BRIGHTNESS >> >> > +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_RD 0x5F >> >> MIPI_DCS_GET_CABC_MIN_BRIGHTNESS >> >> > +#define MIPI_DCS_CABC_CONTROL_RD 0x56 >> >> MIPI_DCS_GET_POWER_SAVE >> >> > +#define MIPI_DCS_CABC_CONTROL_BRIGHT_RD 0x54 >> >> MIPI_DCS_GET_CONTROL_DISPLAY >> >> > +#define MIPI_DCS_CABC_LEVEL_WR 0x51 >> >> MIPI_DCS_SET_DISPLAY_BRIGHTNESS >> >> > +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_WR 0x5E >> >> MIPI_DCS_SET_CABC_MIN_BRIGHTNESS >> >> > +#define MIPI_DCS_CABC_CONTROL_WR 0x55 >> >> MIPI_DCS_WRITE_POWER_SAVE >> >> > +#define MIPI_DCS_CABC_CONTROL_BRIGHT_WR 0x53 >> >> MIPI_DCS_WRITE_CONTROL_DISPLAY >> >> > + >> > +static u32 cabc_get_backlight(struct intel_connector *connector) { >> struct intel_encoder *encoder = connector->encoder; >> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >> >> Same for all functions below. >> >> > + 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; >> > + intel_dsi = enc_to_intel_dsi(&encoder->base); >> > + >> > + 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]; >> >> Hmm, it's up to the manufacturer whether the this is 8 or 16 bits. I guess >> you're just assuming 8 bits? I wonder if this should be more generic wrt 8 vs. >> 16 bits. >> > [Deepak, M] Need to think on how to make it generic wrt 8 vs 16 bits, > problem here is to somehow we have to identify the no of parameters or > we have to pass this info in VBT... Look at the maximum value to decide? That's what needs to be set in setup if the size changes, so no need to add another variable. BR, Jani. >> > +} >> > + >> > +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; >> > + intel_dsi = enc_to_intel_dsi(&encoder->base); >> > + >> > + 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); >> >> Please use mipi_dsi_dcs_write(). Same for all functions below. >> >> > + } >> > +} >> > + >> > +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; >> > + intel_dsi = enc_to_intel_dsi(&encoder->base); >> > + >> > + cabc_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 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; >> > + intel_dsi = enc_to_intel_dsi(&encoder->base); >> > + >> > + 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); >> > + } >> > + >> > + cabc_set_backlight(connector, panel->backlight.level); } >> > + >> > +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; >> > + } >> >> None of the above is needed. >> >> dev_priv->vbt.backlight.present is checked higher level in >> intel_panel_setup_backlight(), and panel->backlight.present = true is set >> there as well if this hook returns 0. >> >> > + >> > + panel->backlight.max = CABC_MAX_VALUE; >> > + panel->backlight.level = CABC_MAX_VALUE; >> > + >> > + return 0; >> > +} >> > + >> > +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector >> > +*intel_connector) { >> > + struct drm_device *dev = intel_connector->base.dev; >> > + struct drm_i915_private *dev_priv = dev->dev_private; >> > + struct intel_encoder *encoder = intel_connector->encoder; >> > + struct intel_panel *panel = &intel_connector->panel; >> > + >> > + if (!dev_priv->vbt.dsi.config->cabc_supported) >> > + return -EINVAL; >> >> -ENODEV seems more appropriate. >> >> > + >> > + if (encoder->type != INTEL_OUTPUT_DSI) { >> > + DRM_ERROR("Use DSI encoder for CABC\n"); >> > + return -EINVAL; >> > + } >> >> if (WARN_ON(encoder->type != INTEL_OUTPUT_DSI)) >> return -EINVAL; >> >> > + >> > + 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; >> > + >> > + return 0; >> > +} >> > diff --git a/drivers/gpu/drm/i915/intel_panel.c >> > b/drivers/gpu/drm/i915/intel_panel.c >> > index 8c8996f..0f9bf80 100644 >> > --- a/drivers/gpu/drm/i915/intel_panel.c >> > +++ b/drivers/gpu/drm/i915/intel_panel.c >> > @@ -1718,6 +1718,10 @@ intel_panel_init_backlight_funcs(struct >> intel_panel *panel) >> > container_of(panel, struct intel_connector, panel); >> > struct drm_i915_private *dev_priv = to_i915(connector->base.dev); >> > >> > + if (connector->base.connector_type == >> DRM_MODE_CONNECTOR_DSI && >> > + intel_dsi_cabc_init_backlight_funcs(connector) == 0) >> >> Indentation is not quite right. >> >> > + return; >> > + >> > if (IS_BROXTON(dev_priv)) { >> > panel->backlight.setup = bxt_setup_backlight; >> > panel->backlight.enable = bxt_enable_backlight; >> >> -- >> Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control 2016-03-23 11:50 ` Jani Nikula @ 2016-03-23 11:51 ` Jani Nikula 0 siblings, 0 replies; 10+ messages in thread From: Jani Nikula @ 2016-03-23 11:51 UTC (permalink / raw) To: Deepak, M, intel-gfx; +Cc: Adebisi, YetundeX, Vetter, Daniel On Wed, 23 Mar 2016, Jani Nikula <jani.nikula@intel.com> wrote: > [ text/plain ] > On Wed, 23 Mar 2016, "Deepak, M" <m.deepak@intel.com> wrote: >>> -----Original Message----- >>> From: Nikula, Jani >>> Sent: Tuesday, March 22, 2016 7:19 PM >>> To: Deepak, M <m.deepak@intel.com>; intel-gfx@lists.freedesktop.org >>> Cc: Deepak, M <m.deepak@intel.com>; Vetter, Daniel >>> <daniel.vetter@intel.com>; Adebisi, YetundeX >>> <yetundex.adebisi@intel.com> >>> Subject: Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control >>> >>> On Tue, 22 Mar 2016, 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. >>> > >>> > v2: Moving the CABC bkl functions to new file.(Jani) >>> > >>> > v3: Rebase >>> > >>> > v4: Rebase >>> > >>> > 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/Makefile | 1 + >>> > drivers/gpu/drm/i915/i915_drv.h | 2 +- >>> > drivers/gpu/drm/i915/intel_dsi.c | 19 +++- >>> > drivers/gpu/drm/i915/intel_dsi.h | 4 + >>> > drivers/gpu/drm/i915/intel_dsi_cabc.c | 179 >>> ++++++++++++++++++++++++++++++++++ >>> > drivers/gpu/drm/i915/intel_panel.c | 4 + >>> > 6 files changed, 206 insertions(+), 3 deletions(-) create mode >>> > 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c >>> > >>> > diff --git a/drivers/gpu/drm/i915/Makefile >>> > b/drivers/gpu/drm/i915/Makefile index 7ffb51b..065c410 100644 >>> > --- a/drivers/gpu/drm/i915/Makefile >>> > +++ b/drivers/gpu/drm/i915/Makefile >>> > @@ -83,6 +83,7 @@ i915-y += dvo_ch7017.o \ >>> > intel_dp_mst.o \ >>> > intel_dp.o \ >>> > intel_dsi.o \ >>> > + intel_dsi_cabc.o \ >>> > intel_dsi_panel_vbt.o \ >>> > intel_dsi_pll.o \ >>> > intel_dvo.o \ >>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h >>> > b/drivers/gpu/drm/i915/i915_drv.h index 050d860..d196404 100644 >>> > --- a/drivers/gpu/drm/i915/i915_drv.h >>> > +++ b/drivers/gpu/drm/i915/i915_drv.h >>> > @@ -3489,7 +3489,7 @@ void intel_sbi_write(struct drm_i915_private >>> *dev_priv, u16 reg, u32 value, >>> > enum intel_sbi_destination destination); >>> > u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg); >>> > void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 >>> > val); >>> > - >>> > +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector >>> > +*intel_connector); >>> >>> This probably fits better in intel_drv.h under a /* intel_dsi_cabc.c */ >>> comment, see the file for examples. >>> >>> > int intel_gpu_freq(struct drm_i915_private *dev_priv, int val); int >>> > intel_freq_opcode(struct drm_i915_private *dev_priv, int val); >>> > >>> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c >>> > b/drivers/gpu/drm/i915/intel_dsi.c >>> > index 456676c..7aa707f 100644 >>> > --- a/drivers/gpu/drm/i915/intel_dsi.c >>> > +++ b/drivers/gpu/drm/i915/intel_dsi.c >>> > @@ -1209,10 +1209,25 @@ void intel_dsi_init(struct drm_device *dev) >>> > else >>> > intel_encoder->crtc_mask = BIT(PIPE_B); >>> > >>> > - if (dev_priv->vbt.dsi.config->dual_link) >>> > + if (dev_priv->vbt.dsi.config->dual_link) { >>> > intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C); >>> > - else >>> > + switch (dev_priv->vbt.dsi.config->dl_cabc_port) { >>> > + case CABC_PORT_A: >>> > + intel_dsi->bkl_dcs_ports = BIT(PORT_A); >>> > + break; >>> > + case CABC_PORT_C: >>> > + intel_dsi->bkl_dcs_ports = BIT(PORT_C); >>> > + break; >>> > + case CABC_PORT_A_AND_C: >>> > + intel_dsi->bkl_dcs_ports = BIT(PORT_A) | >>> BIT(PORT_C); >>> > + break; >>> > + default: >>> > + DRM_ERROR("Unknown MIPI ports for sending >>> DCS\n"); >>> > + } >>> > + } else { >>> > intel_dsi->ports = BIT(port); >>> > + intel_dsi->bkl_dcs_ports = BIT(port); >>> > + } >>> > >>> > /* 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 0e758f1..5c07d59 100644 >>> > --- a/drivers/gpu/drm/i915/intel_dsi.h >>> > +++ b/drivers/gpu/drm/i915/intel_dsi.h >>> > @@ -34,6 +34,10 @@ >>> > #define DSI_DUAL_LINK_FRONT_BACK 1 >>> > #define DSI_DUAL_LINK_PIXEL_ALT 2 >>> > >>> > +#define CABC_PORT_A 0x00 >>> > +#define CABC_PORT_C 0x01 >>> > +#define CABC_PORT_A_AND_C 0x02 >>> > + >>> > struct intel_dsi_host; >>> > >>> > struct intel_dsi { >>> > diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.c >>> > b/drivers/gpu/drm/i915/intel_dsi_cabc.c >>> > new file mode 100644 >>> > index 0000000..d14a669 >>> > --- /dev/null >>> > +++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c >>> > @@ -0,0 +1,179 @@ >>> > +/* >>> > + * Copyright © 2006-2010 Intel Corporation >>> >>> 2016 >>> >>> > + * >>> > + * Permission is hereby granted, free of charge, to any person >>> > + obtaining a >>> > + * copy of this software and associated documentation files (the >>> > + "Software"), >>> > + * to deal in the Software without restriction, including without >>> > + limitation >>> > + * the rights to use, copy, modify, merge, publish, distribute, >>> > + sublicense, >>> > + * and/or sell copies of the Software, and to permit persons to whom >>> > + the >>> > + * Software is furnished to do so, subject to the following conditions: >>> > + * >>> > + * The above copyright notice and this permission notice (including >>> > + the next >>> > + * paragraph) shall be included in all copies or substantial portions >>> > + of the >>> > + * Software. >>> > + * >>> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY >>> KIND, >>> > + EXPRESS OR >>> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>> > + MERCHANTABILITY, >>> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN >>> NO EVENT >>> > + SHALL >>> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, >>> DAMAGES >>> > + OR OTHER >>> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >>> OTHERWISE, >>> > + ARISING >>> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE >>> OR >>> > + OTHER >>> > + * DEALINGS IN THE SOFTWARE. >>> > + * >>> > + * Author: Deepak M <m.deepak at intel.com> */ >>> > + >>> > +#include "intel_drv.h" >>> > +#include "intel_dsi.h" >>> > +#include "i915_drv.h" >>> > +#include <drm/drm_mipi_dsi.h> >>> > + >>> > +#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_MAX_VALUE 0xFF >>> > + >>> >>> The defines below should be added to the relevant enum in >>> include/video/mipi_display.h using the MIPI DCS specification names. I'll >>> copy them below. Please create a separate prep patch and send it to dri- >>> devel. >>> >>> > +#define MIPI_DCS_CABC_LEVEL_RD 0x52 >>> >>> MIPI_DCS_GET_DISPLAY_BRIGHTNESS >>> >>> > +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_RD 0x5F >>> >>> MIPI_DCS_GET_CABC_MIN_BRIGHTNESS >>> >>> > +#define MIPI_DCS_CABC_CONTROL_RD 0x56 >>> >>> MIPI_DCS_GET_POWER_SAVE >>> >>> > +#define MIPI_DCS_CABC_CONTROL_BRIGHT_RD 0x54 >>> >>> MIPI_DCS_GET_CONTROL_DISPLAY >>> >>> > +#define MIPI_DCS_CABC_LEVEL_WR 0x51 >>> >>> MIPI_DCS_SET_DISPLAY_BRIGHTNESS >>> >>> > +#define MIPI_DCS_CABC_MIN_BRIGHTNESS_WR 0x5E >>> >>> MIPI_DCS_SET_CABC_MIN_BRIGHTNESS >>> >>> > +#define MIPI_DCS_CABC_CONTROL_WR 0x55 >>> >>> MIPI_DCS_WRITE_POWER_SAVE >>> >>> > +#define MIPI_DCS_CABC_CONTROL_BRIGHT_WR 0x53 >>> >>> MIPI_DCS_WRITE_CONTROL_DISPLAY >>> >>> > + >>> > +static u32 cabc_get_backlight(struct intel_connector *connector) { >>> struct intel_encoder *encoder = connector->encoder; >>> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); >>> >>> Same for all functions below. >>> >>> > + 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; >>> > + intel_dsi = enc_to_intel_dsi(&encoder->base); >>> > + >>> > + 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]; >>> >>> Hmm, it's up to the manufacturer whether the this is 8 or 16 bits. I guess >>> you're just assuming 8 bits? I wonder if this should be more generic wrt 8 vs. >>> 16 bits. >>> >> [Deepak, M] Need to think on how to make it generic wrt 8 vs 16 bits, >> problem here is to somehow we have to identify the no of parameters or >> we have to pass this info in VBT... > > Look at the maximum value to decide? That's what needs to be set in > setup if the size changes, so no need to add another variable. Or just ignore this part for now, and go for either 8 or 16 bits, not this sometimes this sometimes that. > > BR, > Jani. > >>> > +} >>> > + >>> > +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; >>> > + intel_dsi = enc_to_intel_dsi(&encoder->base); >>> > + >>> > + 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); >>> >>> Please use mipi_dsi_dcs_write(). Same for all functions below. >>> >>> > + } >>> > +} >>> > + >>> > +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; >>> > + intel_dsi = enc_to_intel_dsi(&encoder->base); >>> > + >>> > + cabc_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 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; >>> > + intel_dsi = enc_to_intel_dsi(&encoder->base); >>> > + >>> > + 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); >>> > + } >>> > + >>> > + cabc_set_backlight(connector, panel->backlight.level); } >>> > + >>> > +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; >>> > + } >>> >>> None of the above is needed. >>> >>> dev_priv->vbt.backlight.present is checked higher level in >>> intel_panel_setup_backlight(), and panel->backlight.present = true is set >>> there as well if this hook returns 0. >>> >>> > + >>> > + panel->backlight.max = CABC_MAX_VALUE; >>> > + panel->backlight.level = CABC_MAX_VALUE; >>> > + >>> > + return 0; >>> > +} >>> > + >>> > +int intel_dsi_cabc_init_backlight_funcs(struct intel_connector >>> > +*intel_connector) { >>> > + struct drm_device *dev = intel_connector->base.dev; >>> > + struct drm_i915_private *dev_priv = dev->dev_private; >>> > + struct intel_encoder *encoder = intel_connector->encoder; >>> > + struct intel_panel *panel = &intel_connector->panel; >>> > + >>> > + if (!dev_priv->vbt.dsi.config->cabc_supported) >>> > + return -EINVAL; >>> >>> -ENODEV seems more appropriate. >>> >>> > + >>> > + if (encoder->type != INTEL_OUTPUT_DSI) { >>> > + DRM_ERROR("Use DSI encoder for CABC\n"); >>> > + return -EINVAL; >>> > + } >>> >>> if (WARN_ON(encoder->type != INTEL_OUTPUT_DSI)) >>> return -EINVAL; >>> >>> > + >>> > + 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; >>> > + >>> > + return 0; >>> > +} >>> > diff --git a/drivers/gpu/drm/i915/intel_panel.c >>> > b/drivers/gpu/drm/i915/intel_panel.c >>> > index 8c8996f..0f9bf80 100644 >>> > --- a/drivers/gpu/drm/i915/intel_panel.c >>> > +++ b/drivers/gpu/drm/i915/intel_panel.c >>> > @@ -1718,6 +1718,10 @@ intel_panel_init_backlight_funcs(struct >>> intel_panel *panel) >>> > container_of(panel, struct intel_connector, panel); >>> > struct drm_i915_private *dev_priv = to_i915(connector->base.dev); >>> > >>> > + if (connector->base.connector_type == >>> DRM_MODE_CONNECTOR_DSI && >>> > + intel_dsi_cabc_init_backlight_funcs(connector) == 0) >>> >>> Indentation is not quite right. >>> >>> > + return; >>> > + >>> > if (IS_BROXTON(dev_priv)) { >>> > panel->backlight.setup = bxt_setup_backlight; >>> > panel->backlight.enable = bxt_enable_backlight; >>> >>> -- >>> Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* ✗ Fi.CI.BAT: failure for CABC patch list (rev4) 2016-03-22 5:41 [MIPI CABC 0/2] CABC patch list Deepak M 2016-03-22 5:41 ` [MIPI CABC 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT Deepak M 2016-03-22 5:41 ` [MIPI CABC 2/2] drm/i915: CABC support for backlight control Deepak M @ 2016-03-22 10:04 ` Patchwork 2 siblings, 0 replies; 10+ messages in thread From: Patchwork @ 2016-03-22 10:04 UTC (permalink / raw) To: Deepak M; +Cc: intel-gfx == Series Details == Series: CABC patch list (rev4) URL : https://patchwork.freedesktop.org/series/801/ State : failure == Summary == Series 801v4 CABC patch list http://patchwork.freedesktop.org/api/1.0/series/801/revisions/4/mbox/ Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (bsw-nuc-2) Test gem_sync: Subgroup basic-bsd: pass -> DMESG-FAIL (ilk-hp8440p) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> DMESG-WARN (hsw-brixbox) pass -> DMESG-WARN (bdw-ultra) Subgroup basic-plain-flip: dmesg-warn -> PASS (bdw-ultra) Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: dmesg-warn -> PASS (bsw-nuc-2) Test pm_rpm: Subgroup basic-pci-d3-state: fail -> DMESG-FAIL (snb-x220t) pass -> DMESG-WARN (snb-dellxps) pass -> DMESG-WARN (bsw-nuc-2) Subgroup basic-rte: dmesg-warn -> PASS (snb-x220t) dmesg-warn -> PASS (snb-dellxps) dmesg-warn -> PASS (byt-nuc) UNSTABLE bdw-nuci7 total:192 pass:180 dwarn:0 dfail:0 fail:0 skip:12 bdw-ultra total:192 pass:170 dwarn:1 dfail:0 fail:0 skip:21 bsw-nuc-2 total:192 pass:153 dwarn:2 dfail:0 fail:0 skip:37 byt-nuc total:192 pass:157 dwarn:0 dfail:0 fail:0 skip:35 hsw-brixbox total:192 pass:169 dwarn:1 dfail:0 fail:0 skip:22 ilk-hp8440p total:192 pass:127 dwarn:1 dfail:1 fail:0 skip:63 ivb-t430s total:192 pass:167 dwarn:0 dfail:0 fail:0 skip:25 skl-i5k-2 total:192 pass:169 dwarn:0 dfail:0 fail:0 skip:23 skl-i7k-2 total:192 pass:169 dwarn:0 dfail:0 fail:0 skip:23 snb-dellxps total:192 pass:157 dwarn:1 dfail:0 fail:0 skip:34 snb-x220t total:192 pass:158 dwarn:0 dfail:1 fail:0 skip:33 Results at /archive/results/CI_IGT_test/Patchwork_1668/ 4b39223f6e3bef4dfa678f7239dcd87c38e20e96 drm-intel-nightly: 2016y-03m-21d-18h-43m-18s UTC integration manifest d932b2d799c9053e0db544f3ca42d59f4fbdcbd2 drm/i915: CABC support for backlight control 9e5099ce3194aa3adf5b87470aacb7a4afdd6514 drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* [MIPI CABC 0/2] CABC patch list @ 2015-11-16 20:18 Deepak M 2015-11-16 20:18 ` [MIPI CABC 2/2] drm/i915: CABC support for backlight control Deepak M 0 siblings, 1 reply; 10+ messages in thread From: Deepak M @ 2015-11-16 20:18 UTC (permalink / raw) To: intel-gfx; +Cc: Deepak M, Jani Nikula, Yetunde Adebisi, Daniel Vetter 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [MIPI CABC 2/2] drm/i915: CABC support for backlight control 2015-11-16 20:18 [MIPI CABC 0/2] CABC patch list Deepak M @ 2015-11-16 20:18 ` Deepak M 2015-11-25 12:26 ` Jani Nikula 0 siblings, 1 reply; 10+ messages in thread From: Deepak M @ 2015-11-16 20:18 UTC (permalink / raw) To: intel-gfx; +Cc: Deepak M, Jani Nikula, Yetunde Adebisi, Daniel Vetter 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [MIPI CABC 2/2] drm/i915: CABC support for backlight control 2015-11-16 20:18 ` [MIPI CABC 2/2] drm/i915: CABC support for backlight control Deepak M @ 2015-11-25 12:26 ` Jani Nikula 0 siblings, 0 replies; 10+ messages in thread From: Jani Nikula @ 2015-11-25 12:26 UTC (permalink / raw) To: intel-gfx; +Cc: Deepak M, Daniel Vetter, Yetunde Adebisi 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-03-23 11:51 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-22 5:41 [MIPI CABC 0/2] CABC patch list Deepak M 2016-03-22 5:41 ` [MIPI CABC 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT Deepak M 2016-03-22 5:41 ` [MIPI CABC 2/2] drm/i915: CABC support for backlight control Deepak M 2016-03-22 13:48 ` Jani Nikula 2016-03-23 10:49 ` Deepak, M 2016-03-23 11:50 ` Jani Nikula 2016-03-23 11:51 ` Jani Nikula 2016-03-22 10:04 ` ✗ Fi.CI.BAT: failure for CABC patch list (rev4) Patchwork -- strict thread matches above, loose matches on Subject: below -- 2015-11-16 20:18 [MIPI CABC 0/2] CABC patch list Deepak M 2015-11-16 20:18 ` [MIPI CABC 2/2] drm/i915: CABC support for backlight control Deepak M 2015-11-25 12:26 ` Jani Nikula
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.