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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 11 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.
>
> 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@xxxxxxxxx>
> ---
>  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@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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux