From: Jani Nikula <jani.nikula@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Deepak M <m.deepak@intel.com>
Subject: Re: [RFC CABC v3 PATCH 2/2] drm/i915: CABC support for backlight control
Date: Fri, 11 Sep 2015 14:28:02 +0300 [thread overview]
Message-ID: <87zj0tjiv1.fsf@intel.com> (raw)
In-Reply-To: <1441946868-4985-3-git-send-email-m.deepak@intel.com>
On Fri, 11 Sep 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.
>
> v2:
> - Created a new backlight driver for cabc, which will be registered
> only when it cabc is supported by panel. (Daniel Vetter)
I don't know what Daniel's been telling you, but I think this does need
to get bolted into the regular backlight control mechanism. We'll also
need eDP specific backlight control, and there's the VLV/CVH pwm driver
absed backlight control, so we need to cover this more gracefully than
an if-else ladder anyway.
My idea all along has been that the backlight hooks in dev_priv.display
need to be moved to connectors (probably intel_panel), and connector
backlight setup can do what they want with these. All the boilerplate
code for sysfs and scaling and so on would be there already.
I do not approve of the approach here.
BR,
Jani.
> 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 \
> 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
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-09-11 11:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-11 4:47 [RFC CABC v3 PATCH 0/2] CABC patch list Deepak M
2015-09-11 4:47 ` [RFC CABC v3 PATCH 1/2] drm/i915: Parsing the PWM cntrl and CABC ON/OFF fileds in VBT Deepak M
2015-09-11 4:47 ` [RFC CABC v3 PATCH 2/2] drm/i915: CABC support for backlight control Deepak M
2015-09-11 11:28 ` Jani Nikula [this message]
2015-09-14 9:39 ` Daniel Vetter
2015-09-14 10:10 ` Jani Nikula
2015-09-14 12:34 ` Jani Nikula
2015-09-14 13:19 ` Daniel Vetter
2015-09-14 13:59 ` Jani Nikula
2015-09-14 9:41 ` Daniel Vetter
2015-09-14 9:41 ` Deepak, M
2015-09-14 9:53 ` [PATCH] " Deepak M
2015-09-14 12:20 ` Jani Nikula
2015-09-14 15:31 ` Deepak, M
2015-09-15 7:03 ` Jani Nikula
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zj0tjiv1.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=m.deepak@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.