Re: [RFC CABC v3 PATCH 2/2] drm/i915: CABC support for backlight control

Deepak, M
2015-09-14T09:42:02Z

>-----Original Message-----
>From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
>Vetter
>Sent: Monday, September 14, 2015 3:12 PM
>To: Deepak, M
>Cc: intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [RFC CABC v3 PATCH 2/2] drm/i915: CABC support for
>backlight control
>
>On Fri, Sep 11, 2015 at 10:17:48AM +0530, Deepak M 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:
>> - 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@intel.com>
>> ---
>> drivers/gpu/drm/i915/Makefile | 1 +
>> drivers/gpu/drm/i915/intel_dsi.c | 18 +++++++++++++++---
>> drivers/gpu/drm/i915/intel_panel.c | 23 +++++++++++++++++++----
>> include/video/mipi_display.h | 8 ++++++++
>> 4 files changed, 43 insertions(+), 7 deletions(-)
>>
>> 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 \
>
>You've forgotten to git add intel_dsi_cabc.c. Makes reviewing things a bit hard
>;-) -Daniel
[Deepak M] Extremely sorry, git am didn't worked had did patch -p1 but got forgot to do a git am :), will update the patch.
>
>> 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);
>> }
>>
>> 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_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,
>> };
>>
>> /* MIPI DCS pixel formats */
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch