On Mon, 14 Sep 2015, Deepak M <m.deepak@xxxxxxxxx> 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 think CABC is a confusing name for this. At the high level, this
should be called, say, DCS backlight control. I think CABC is a special
case of backlight control in the DSI panel; there are panels with
brightness control via DCS *without* CABC.
See further comments inline.
>
> v2:
> - Created a new backlight driver for cabc, which will be registered
> only when it cabc is supported by panel. (Daniel Vetter)
> v3:
> - Use for_each_dsi_port macro for handling port C also (Gaurav)
> - Rebase
>
> Signed-off-by: Deepak M <m.deepak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/intel_dsi.c | 18 +-
> drivers/gpu/drm/i915/intel_dsi_cabc.c | 310 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_dsi_cabc.h | 46 +++++
> drivers/gpu/drm/i915/intel_panel.c | 23 ++-
> include/video/mipi_display.h | 8 +
> 6 files changed, 399 insertions(+), 7 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.c
> create mode 100644 drivers/gpu/drm/i915/intel_dsi_cabc.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 44d290a..d87c690 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -82,6 +82,7 @@ i915-y += dvo_ch7017.o \
> intel_dsi.o \
> intel_dsi_panel_vbt.o \
> intel_dsi_pll.o \
> + intel_dsi_cabc.o \
> intel_dvo.o \
> intel_hdmi.o \
> intel_i2c.o \
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 781c267..1d98ed8 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -35,6 +35,7 @@
> #include "i915_drv.h"
> #include "intel_drv.h"
> #include "intel_dsi.h"
> +#include "intel_dsi_cabc.h"
>
> static const struct {
> u16 panel_id;
> @@ -398,7 +399,10 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
> intel_dsi_port_enable(encoder);
> }
>
> - intel_panel_enable_backlight(intel_dsi->attached_connector);
> + if (dev_priv->vbt.dsi.config->cabc_supported)
> + cabc_enable_backlight(intel_dsi->attached_connector);
> + else
> + intel_panel_enable_backlight(intel_dsi->attached_connector);
See my patch at [1] for how I think all of this should be handled.
[1] http://mid.gmane.org/cover.1442227790.git.jani.nikula@xxxxxxxxx
> }
>
> static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> @@ -458,12 +462,17 @@ static void intel_dsi_enable_nop(struct intel_encoder *encoder)
>
> static void intel_dsi_pre_disable(struct intel_encoder *encoder)
> {
> + struct drm_device *dev = encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> enum port port;
>
> DRM_DEBUG_KMS("\n");
>
> - intel_panel_disable_backlight(intel_dsi->attached_connector);
> + if (dev_priv->vbt.dsi.config->cabc_supported)
> + cabc_disable_backlight(intel_dsi->attached_connector);
> + else
> + intel_panel_disable_backlight(intel_dsi->attached_connector);
>
> if (is_vid_mode(intel_dsi)) {
> /* Send Shutdown command to the panel in LP mode */
> @@ -1133,7 +1142,10 @@ void intel_dsi_init(struct drm_device *dev)
> }
>
> intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
> - intel_panel_setup_backlight(connector, INVALID_PIPE);
> + if (dev_priv->vbt.dsi.config->cabc_supported)
> + cabc_setup_backlight(connector, INVALID_PIPE);
> + else
> + intel_panel_setup_backlight(connector, INVALID_PIPE);
>
> return;
>
> 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..c2fcd95
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_dsi_cabc.c
> @@ -0,0 +1,310 @@
> +/*
> + * 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@xxxxxxxxx>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/moduleparam.h>
> +#include "intel_drv.h"
> +#include "intel_dsi.h"
> +#include "i915_drv.h"
> +#include "intel_dsi_cabc.h"
> +#include <video/mipi_display.h>
> +#include <drm/drm_mipi_dsi.h>
> +
> +static inline enum port get_cabc_port(struct intel_dsi *intel_dsi)
> +{
> + if (intel_dsi->dl_cabc_port == CABC_PORT_C)
> + return PORT_C;
> + else
> + return PORT_A;
> +}
> +
> +int cabc_setup_backlight(struct drm_connector *connector,
> + enum pipe unused)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_connector *intel_connector = to_intel_connector(connector);
> + struct intel_panel *panel = &intel_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;
> +}
> +
> +u32 cabc_get_backlight(struct intel_connector *connector)
> +{
> + struct drm_device *dev = connector->base.dev;
> + struct intel_dsi *intel_dsi = NULL;
> + struct drm_crtc *crtc = NULL;
> + struct intel_encoder *encoder = NULL;
> + struct mipi_dsi_device *dsi_device;
> + u8 data[2] = {0};
> + enum port port;
> + u32 ret;
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->type == INTEL_OUTPUT_DSI) {
> + intel_dsi = enc_to_intel_dsi(&encoder->base);
> + break;
> + }
> + }
> + if (intel_dsi != NULL)
> + break;
> + }
You can get at intel_dsi via connector->encoder. No need for this kind
of loops.
Same for similar loops below.
> +
> + for_each_dsi_port(port, intel_dsi->ports) {
> + if (intel_dsi->dual_link)
> + port = get_cabc_port(intel_dsi);
> +
> + dsi_device = intel_dsi->dsi_hosts[port]->device;
> + mipi_dsi_dcs_read(dsi_device, MIPI_DCS_CABC_LEVEL_RD, data, 2);
> +
> + if (intel_dsi->dual_link &&
> + !(intel_dsi->dl_cabc_port == CABC_PORT_A_AND_C))
> + break;
> + }
That is a really confusing way to do the dcs command on one or more
ports. How about making dl_cabc_port (hopefully renamed to something
referring to dcs rather than cabc) a bitmap of ports similar to
intel_dsi->ports, so you can make this loop simply:
for_each_dsi_port(port, dcs_backlight_ports)
...
There'd be no need for special casing and conditional breaks out of the
loop or anything.
Same for similar loops below.
> +
> + ret = data[1];
> +
> + return ret;
> +}
> +
> +void cabc_set_backlight(struct intel_connector *connector, u32 level)
> +{
> + struct drm_device *dev = connector->base.dev;
> + struct intel_dsi *intel_dsi = NULL;
> + struct drm_crtc *crtc = NULL;
> + struct intel_encoder *encoder = NULL;
> + struct mipi_dsi_device *dsi_device;
> + u8 data[2] = {0};
> + enum port port;
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->type == INTEL_OUTPUT_DSI) {
> + intel_dsi = enc_to_intel_dsi(&encoder->base);
> + break;
> + }
> + }
> + if (intel_dsi != NULL)
> + break;
> + }
> +
> + for_each_dsi_port(port, intel_dsi->ports) {
> + if (intel_dsi->dual_link)
> + port = get_cabc_port(intel_dsi);
> +
> + 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);
> +
> + if (intel_dsi->dual_link &&
> + !(intel_dsi->dl_cabc_port == CABC_PORT_A_AND_C))
> + break;
> + }
> +}
> +
> +void cabc_enable_backlight(struct intel_connector *connector)
> +{
> + struct drm_device *dev = connector->base.dev;
> + struct intel_dsi *intel_dsi = NULL;
> + struct drm_crtc *crtc = 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};
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->type == INTEL_OUTPUT_DSI) {
> + intel_dsi = enc_to_intel_dsi(&encoder->base);
> + break;
> + }
> + }
> + if (intel_dsi != NULL)
> + break;
> + }
> +
> + for_each_dsi_port(port, intel_dsi->ports) {
> + if (intel_dsi->dual_link)
> + port = get_cabc_port(intel_dsi);
> +
> + 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);
> +
> + if (intel_dsi->dual_link &&
> + !(intel_dsi->dl_cabc_port == CABC_PORT_A_AND_C))
> + break;
> + }
> +
> + cabc_set_backlight(connector, panel->backlight.level);
> +}
> +
> +void cabc_disable_backlight(struct intel_connector *connector)
> +{
> + struct drm_device *dev = connector->base.dev;
> + struct intel_dsi *intel_dsi = NULL;
> + struct drm_crtc *crtc = NULL;
> + struct intel_encoder *encoder = NULL;
> + struct mipi_dsi_device *dsi_device;
> + enum port port;
> + u8 data[2] = {0};
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> + for_each_encoder_on_crtc(dev, crtc, encoder) {
> + if (encoder->type == INTEL_OUTPUT_DSI) {
> + intel_dsi = enc_to_intel_dsi(&encoder->base);
> + break;
> + }
> + }
> + if (intel_dsi != NULL)
> + break;
> + }
> +
> + cabc_set_backlight(connector, 0);
> +
> + for_each_dsi_port(port, intel_dsi->ports) {
> + if (intel_dsi->dual_link)
> + port = get_cabc_port(intel_dsi);
> + 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);
> +
> + if (intel_dsi->dual_link &&
> + !(intel_dsi->dl_cabc_port == CABC_PORT_A_AND_C))
> + break;
> + }
> +}
> +static int cabc_backlight_device_update_status(struct backlight_device *bd)
> +{
> + struct intel_connector *connector = bl_get_data(bd);
> + struct drm_device *dev = connector->base.dev;
> +
> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> + DRM_DEBUG_KMS("updating intel_backlight, brightness=%d/%d\n",
> + bd->props.brightness, bd->props.max_brightness);
> +
> + cabc_set_backlight(connector, bd->props.brightness);
> +
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +
> + return 0;
> +}
> +
> +static int cabc_backlight_device_get_brightness(struct backlight_device *bd)
> +{
> + struct intel_connector *connector = bl_get_data(bd);
> + struct drm_device *dev = connector->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int ret;
> +
> + intel_runtime_pm_get(dev_priv);
> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> +
> + ret = cabc_get_backlight(connector);
> +
> + drm_modeset_unlock(&dev->mode_config.connection_mutex);
> + intel_runtime_pm_put(dev_priv);
> +
> + return ret;
> +}
> +
> +static const struct backlight_ops cabc_backlight_device_ops = {
> + .update_status = cabc_backlight_device_update_status,
> + .get_brightness = cabc_backlight_device_get_brightness,
> +};
> +
> +int cabc_backlight_device_register(struct intel_connector *connector)
> +{
> + struct intel_panel *panel = &connector->panel;
> + struct backlight_properties props;
> +
> + if (WARN_ON(panel->backlight.device))
> + return -ENODEV;
> +
> + if (!panel->backlight.present) {
> + DRM_ERROR("No backlight present\n");
> + return 0;
> + }
> +
> + WARN_ON(panel->backlight.max == 0);
> +
> + memset(&props, 0, sizeof(props));
> + props.type = BACKLIGHT_FIRMWARE;
> + props.max_brightness = panel->backlight.max;
> + props.brightness = props.max_brightness;
> +
> + /*
> + * Note: using the same name independent of the connector prevents
> + * registration of multiple backlight devices in the driver.
> + */
> + panel->backlight.device =
> + backlight_device_register("intel_backlight",
> + connector->base.kdev,
> + connector,
> + &cabc_backlight_device_ops, &props);
> +
> + if (IS_ERR(panel->backlight.device)) {
> + DRM_ERROR("Failed to register backlight: %ld\n",
> + PTR_ERR(panel->backlight.device));
> + panel->backlight.device = NULL;
> + return -ENODEV;
> + }
> +
> + DRM_DEBUG_KMS("Connector %s backlight sysfs interface registered\n",
> + connector->base.name);
> +
> + return 0;
> +}
> +
> +void cabc_backlight_device_unregister(struct intel_connector *connector)
> +{
> + struct intel_panel *panel = &connector->panel;
> +
> + if (panel->backlight.device) {
> + backlight_device_unregister(panel->backlight.device);
> + panel->backlight.device = NULL;
> + }
> +}
> diff --git a/drivers/gpu/drm/i915/intel_dsi_cabc.h b/drivers/gpu/drm/i915/intel_dsi_cabc.h
> new file mode 100644
> index 0000000..b1a79d9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_dsi_cabc.h
> @@ -0,0 +1,46 @@
> +/*
> + * Copyright © 2013 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@xxxxxxxxx>
> + */
> +
> +extern int cabc_backlight_device_register(struct intel_connector *connector);
> +extern void cabc_backlight_device_unregister(struct intel_connector *connector);
> +void cabc_set_backlight(struct intel_connector *connector, u32 level);
> +u32 cabc_get_backlight(struct intel_connector *connector);
> +void cabc_enable_backlight(struct intel_connector *connector);
> +void cabc_disable_backlight(struct intel_connector *connector);
> +int cabc_setup_backlight(struct drm_connector *connector, enum pipe unused);
> +
> +#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
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e2ab3f6..ff2e586 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_cabc.h"
>
> #define CRC_PMIC_PWM_PERIOD_NS 21333
>
> @@ -1586,15 +1587,29 @@ void intel_panel_fini(struct intel_panel *panel)
> void intel_backlight_register(struct drm_device *dev)
> {
> struct intel_connector *connector;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>
> - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
> - intel_backlight_device_register(connector);
> + list_for_each_entry(connector, &dev->mode_config.connector_list,
> + base.head) {
> + if (connector->encoder->type == INTEL_OUTPUT_DSI &&
> + dev_priv->vbt.dsi.config->cabc_supported)
> + cabc_backlight_device_register(connector);
> + else
> + intel_backlight_device_register(connector);
> + }
> }
>
> void intel_backlight_unregister(struct drm_device *dev)
> {
> struct intel_connector *connector;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>
> - list_for_each_entry(connector, &dev->mode_config.connector_list, base.head)
> - intel_backlight_device_unregister(connector);
> + list_for_each_entry(connector, &dev->mode_config.connector_list,
> + base.head) {
> + if (connector->encoder->type == INTEL_OUTPUT_DSI &&
> + dev_priv->vbt.dsi.config->cabc_supported)
> + cabc_backlight_device_unregister(connector);
> + else
> + intel_backlight_device_unregister(connector);
> + }
> }
> diff --git a/include/video/mipi_display.h b/include/video/mipi_display.h
> index ddcc8ca..5b8eeec 100644
> --- a/include/video/mipi_display.h
> +++ b/include/video/mipi_display.h
> @@ -117,6 +117,14 @@ enum {
> MIPI_DCS_GET_SCANLINE = 0x45,
> MIPI_DCS_READ_DDB_START = 0xA1,
> MIPI_DCS_READ_DDB_CONTINUE = 0xA8,
> + MIPI_DCS_CABC_LEVEL_RD = 0x52,
> + MIPI_DCS_CABC_MIN_BRIGHTNESS_RD = 0x5F,
> + MIPI_DCS_CABC_CONTROL_RD = 0x56,
> + MIPI_DCS_CABC_CONTROL_BRIGHT_RD = 0x54,
> + MIPI_DCS_CABC_LEVEL_WR = 0x51,
> + MIPI_DCS_CABC_MIN_BRIGHTNESS_WR = 0x5E,
> + MIPI_DCS_CABC_CONTROL_WR = 0x55,
> + MIPI_DCS_CABC_CONTROL_BRIGHT_WR = 0x53,
These changes need to be a separate patch. And what's the spec
reference, I can't find these.
BR,
Jani.
> };
>
> /* MIPI DCS pixel formats */
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
