[Intel-gfx] [PATCH 2/7] drm/i915: make backlight functions take a connector

Joe Konno joe.konno at linux.intel.com
Wed Nov 6 18:08:40 CET 2013


On 10/31/2013 09:55 AM, Jani Nikula wrote:
> From: Jesse Barnes <jbarnes at virtuousgeek.org>
>
> On VLV/BYT, backlight controls a per-pipe, so when adjusting the
> backlight we need to pass the correct info.  So make the externally
> visible backlight functions take a connector argument, which can be used
> internally to figure out the pipe backlight to adjust.
>
> v2: make connector pipe lookup check for NULL crtc (Jani)
>      fixup connector check in ASLE code (Jani)
> v3: make sure we take the mode config lock around lookups (Daniel)
> v4: fix double unlock in panel_get_brightness (Daniel)
> v5: push ASLE work into a work queue (Daniel)
> v6: separate ASLE work to a prep patch, rebase (Jani)
>
> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h       |    1 +
>   drivers/gpu/drm/i915/intel_display.c  |   12 +++++
>   drivers/gpu/drm/i915/intel_dp.c       |    5 +-
>   drivers/gpu/drm/i915/intel_drv.h      |    8 +--
>   drivers/gpu/drm/i915/intel_lvds.c     |    9 ++--
>   drivers/gpu/drm/i915/intel_opregion.c |   38 ++++++++++++++-
>   drivers/gpu/drm/i915/intel_panel.c    |   86 +++++++++++++++++++++++----------
>   7 files changed, 122 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6308711..83eda64 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -54,6 +54,7 @@
>   #define DRIVER_DATE		"20080730"
>
>   enum pipe {
> +	INVALID_PIPE = -1,
>   	PIPE_A = 0,
>   	PIPE_B,
>   	PIPE_C,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8f40ae3..606a594 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9858,6 +9858,18 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>   	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
>   }
>
> +enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
> +{
> +	struct drm_encoder *encoder = connector->base.encoder;
> +
> +	WARN_ON(!mutex_is_locked(&connector->base.dev->mode_config.mutex));
> +
> +	if (!encoder)
> +		return INVALID_PIPE;
> +
> +	return to_intel_crtc(encoder->crtc)->pipe;
> +}
> +
>   int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
>   				struct drm_file *file)
>   {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8db1fda..0ca98825 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1249,7 +1249,6 @@ void ironlake_edp_backlight_on(struct intel_dp *intel_dp)
>   	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   	struct drm_device *dev = intel_dig_port->base.base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int pipe = to_intel_crtc(intel_dig_port->base.base.crtc)->pipe;
>   	u32 pp;
>   	u32 pp_ctrl_reg;
>
> @@ -1272,7 +1271,7 @@ void ironlake_edp_backlight_on(struct intel_dp *intel_dp)
>   	I915_WRITE(pp_ctrl_reg, pp);
>   	POSTING_READ(pp_ctrl_reg);
>
> -	intel_panel_enable_backlight(dev, pipe);
> +	intel_panel_enable_backlight(intel_dp->attached_connector);
>   }
>
>   void ironlake_edp_backlight_off(struct intel_dp *intel_dp)
> @@ -1285,7 +1284,7 @@ void ironlake_edp_backlight_off(struct intel_dp *intel_dp)
>   	if (!is_edp(intel_dp))
>   		return;
>
> -	intel_panel_disable_backlight(dev);
> +	intel_panel_disable_backlight(intel_dp->attached_connector);
>
>   	DRM_DEBUG_KMS("\n");
>   	pp = ironlake_get_pp_control(intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9d2624f..1e49aa8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -630,6 +630,7 @@ void intel_connector_attach_encoder(struct intel_connector *connector,
>   struct drm_encoder *intel_best_encoder(struct drm_connector *connector);
>   struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
>   					     struct drm_crtc *crtc);
> +enum pipe intel_get_pipe_from_connector(struct intel_connector *connector);
>   int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
>   				struct drm_file *file_priv);
>   enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
> @@ -802,10 +803,11 @@ void intel_pch_panel_fitting(struct intel_crtc *crtc,
>   void intel_gmch_panel_fitting(struct intel_crtc *crtc,
>   			      struct intel_crtc_config *pipe_config,
>   			      int fitting_mode);
> -void intel_panel_set_backlight(struct drm_device *dev, u32 level, u32 max);
> +void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
> +			       u32 max);
>   int intel_panel_setup_backlight(struct drm_connector *connector);
> -void intel_panel_enable_backlight(struct drm_device *dev, enum pipe pipe);
> -void intel_panel_disable_backlight(struct drm_device *dev);
> +void intel_panel_enable_backlight(struct intel_connector *connector);
> +void intel_panel_disable_backlight(struct intel_connector *connector);
>   void intel_panel_destroy_backlight(struct drm_device *dev);
>   enum drm_connector_status intel_panel_detect(struct drm_device *dev);
>
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index b0ef558..c3b4da7 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -206,7 +206,8 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
>   {
>   	struct drm_device *dev = encoder->base.dev;
>   	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	struct intel_connector *intel_connector =
> +		&lvds_encoder->attached_connector->base;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	u32 ctl_reg, stat_reg;
>
> @@ -225,13 +226,15 @@ static void intel_enable_lvds(struct intel_encoder *encoder)
>   	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
>   		DRM_ERROR("timed out waiting for panel to power on\n");
>
> -	intel_panel_enable_backlight(dev, intel_crtc->pipe);
> +	intel_panel_enable_backlight(intel_connector);
>   }
>
>   static void intel_disable_lvds(struct intel_encoder *encoder)
>   {
>   	struct drm_device *dev = encoder->base.dev;
>   	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> +	struct intel_connector *intel_connector =
> +		&lvds_encoder->attached_connector->base;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	u32 ctl_reg, stat_reg;
>
> @@ -243,7 +246,7 @@ static void intel_disable_lvds(struct intel_encoder *encoder)
>   		stat_reg = PP_STATUS;
>   	}
>
> -	intel_panel_disable_backlight(dev);
> +	intel_panel_disable_backlight(intel_connector);
>
>   	I915_WRITE(ctl_reg, I915_READ(ctl_reg) & ~POWER_TARGET_ON);
>   	if (wait_for((I915_READ(stat_reg) & PP_ON) == 0, 1000))
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 892d520..91b68dc 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -396,7 +396,13 @@ int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
>   static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_encoder *encoder;
> +	struct drm_connector *connector;
> +	struct intel_connector *intel_connector = NULL;
> +	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[0];
>   	struct opregion_asle __iomem *asle = dev_priv->opregion.asle;
> +	u32 ret = 0;
> +	bool found = false;
>
>   	DRM_DEBUG_DRIVER("bclp = 0x%08x\n", bclp);
>
> @@ -407,11 +413,39 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>   	if (bclp > 255)
>   		return ASLC_BACKLIGHT_FAILED;
>
> +	mutex_lock(&dev->mode_config.mutex);
> +	/*
> +	 * Could match the OpRegion connector here instead, but we'd also need
> +	 * to verify the connector could handle a backlight call.
> +	 */
> +	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
> +		if (encoder->crtc == crtc) {
> +			found = true;
> +			break;
> +		}
> +
> +	if (!found) {
> +		ret = ASLC_BACKLIGHT_FAILED;
> +		goto out;
> +	}
> +
> +	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
> +		if (connector->encoder == encoder)
> +			intel_connector = to_intel_connector(connector);
> +
> +	if (!intel_connector) {
> +		ret = ASLC_BACKLIGHT_FAILED;
> +		goto out;
> +	}
> +
>   	DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp);
> -	intel_panel_set_backlight(dev, bclp, 255);
> +	intel_panel_set_backlight(intel_connector, bclp, 255);
>   	iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv);
>
> -	return 0;
> +out:
> +	mutex_unlock(&dev->mode_config.mutex);
> +
> +	return ret;
>   }
>
>   static u32 asle_set_als_illum(struct drm_device *dev, u32 alsi)
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 09b2994..0f1ebc8 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -341,7 +341,7 @@ static int is_backlight_combination_mode(struct drm_device *dev)
>   /* XXX: query mode clock or hardware clock and program max PWM appropriately
>    * when it's 0.
>    */
> -static u32 i915_read_blc_pwm_ctl(struct drm_device *dev)
> +static u32 i915_read_blc_pwm_ctl(struct drm_device *dev, enum pipe pipe)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	u32 val;
> @@ -380,11 +380,12 @@ static u32 i915_read_blc_pwm_ctl(struct drm_device *dev)
>   	return val;
>   }
>
> -static u32 intel_panel_get_max_backlight(struct drm_device *dev)
> +static u32 intel_panel_get_max_backlight(struct drm_device *dev,
> +					 enum pipe pipe)
>   {
>   	u32 max;
>
> -	max = i915_read_blc_pwm_ctl(dev);
> +	max = i915_read_blc_pwm_ctl(dev, pipe);
>
>   	if (HAS_PCH_SPLIT(dev)) {
>   		max >>= 16;
> @@ -410,7 +411,8 @@ MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness "
>   	"to dri-devel at lists.freedesktop.org, if your machine needs it. "
>   	"It will then be included in an upcoming module version.");
>   module_param_named(invert_brightness, i915_panel_invert_brightness, int, 0600);
> -static u32 intel_panel_compute_brightness(struct drm_device *dev, u32 val)
> +static u32 intel_panel_compute_brightness(struct drm_device *dev,
> +					  enum pipe pipe, u32 val)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>
> @@ -419,7 +421,7 @@ static u32 intel_panel_compute_brightness(struct drm_device *dev, u32 val)
>
>   	if (i915_panel_invert_brightness > 0 ||
>   	    dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) {
> -		u32 max = intel_panel_get_max_backlight(dev);
> +		u32 max = intel_panel_get_max_backlight(dev, pipe);
>   		if (max)
>   			return max - val;
>   	}
> @@ -427,7 +429,8 @@ static u32 intel_panel_compute_brightness(struct drm_device *dev, u32 val)
>   	return val;
>   }
>
> -static u32 intel_panel_get_backlight(struct drm_device *dev)
> +static u32 intel_panel_get_backlight(struct drm_device *dev,
> +				     enum pipe pipe)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	u32 val;
> @@ -450,7 +453,7 @@ static u32 intel_panel_get_backlight(struct drm_device *dev)
>   		}
>   	}
>
> -	val = intel_panel_compute_brightness(dev, val);
> +	val = intel_panel_compute_brightness(dev, pipe, val);
>
>   	spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
>
> @@ -466,19 +469,19 @@ static void intel_pch_panel_set_backlight(struct drm_device *dev, u32 level)
>   }
>
>   static void intel_panel_actually_set_backlight(struct drm_device *dev,
> -					       u32 level)
> +					       enum pipe pipe, u32 level)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	u32 tmp;
>
>   	DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);
> -	level = intel_panel_compute_brightness(dev, level);
> +	level = intel_panel_compute_brightness(dev, pipe, level);
>
>   	if (HAS_PCH_SPLIT(dev))
>   		return intel_pch_panel_set_backlight(dev, level);
>
>   	if (is_backlight_combination_mode(dev)) {
> -		u32 max = intel_panel_get_max_backlight(dev);
> +		u32 max = intel_panel_get_max_backlight(dev, pipe);
>   		u8 lbpc;
>
>   		/* we're screwed, but keep behaviour backwards compatible */
> @@ -498,15 +501,21 @@ static void intel_panel_actually_set_backlight(struct drm_device *dev,
>   }
>
>   /* set backlight brightness to level in range [0..max] */
> -void intel_panel_set_backlight(struct drm_device *dev, u32 level, u32 max)
> +void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
> +			       u32 max)
>   {
> +	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = intel_get_pipe_from_connector(connector);
>   	u32 freq;
>   	unsigned long flags;
>
> +	if (pipe == INVALID_PIPE)
> +		return;
> +
>   	spin_lock_irqsave(&dev_priv->backlight.lock, flags);
>
> -	freq = intel_panel_get_max_backlight(dev);
> +	freq = intel_panel_get_max_backlight(dev, pipe);
>   	if (!freq) {
>   		/* we are screwed, bail out */
>   		goto out;
> @@ -523,16 +532,21 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level, u32 max)
>   		dev_priv->backlight.device->props.brightness = level;
>
>   	if (dev_priv->backlight.enabled)
> -		intel_panel_actually_set_backlight(dev, level);
> +		intel_panel_actually_set_backlight(dev, pipe, level);
>   out:
>   	spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
>   }
>
> -void intel_panel_disable_backlight(struct drm_device *dev)
> +void intel_panel_disable_backlight(struct intel_connector *connector)
>   {
> +	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = intel_get_pipe_from_connector(connector);
>   	unsigned long flags;
>
> +	if (pipe == INVALID_PIPE)
> +		return;
> +
>   	/*
>   	 * Do not disable backlight on the vgaswitcheroo path. When switching
>   	 * away from i915, the other client may depend on i915 to handle the
> @@ -547,7 +561,7 @@ void intel_panel_disable_backlight(struct drm_device *dev)
>   	spin_lock_irqsave(&dev_priv->backlight.lock, flags);
>
>   	dev_priv->backlight.enabled = false;
> -	intel_panel_actually_set_backlight(dev, 0);
> +	intel_panel_actually_set_backlight(dev, pipe, 0);
>
>   	if (INTEL_INFO(dev)->gen >= 4) {
>   		uint32_t reg, tmp;
> @@ -566,20 +580,25 @@ void intel_panel_disable_backlight(struct drm_device *dev)
>   	spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
>   }
>
> -void intel_panel_enable_backlight(struct drm_device *dev,
> -				  enum pipe pipe)
> +void intel_panel_enable_backlight(struct intel_connector *connector)
>   {
> +	struct drm_device *dev = connector->base.dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = intel_get_pipe_from_connector(connector);
>   	enum transcoder cpu_transcoder =
>   		intel_pipe_to_cpu_transcoder(dev_priv, pipe);
>   	unsigned long flags;
>
> +	if (pipe == INVALID_PIPE)
> +		return;
> +
>   	DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
>
>   	spin_lock_irqsave(&dev_priv->backlight.lock, flags);
>
>   	if (dev_priv->backlight.level == 0) {
> -		dev_priv->backlight.level = intel_panel_get_max_backlight(dev);
> +		dev_priv->backlight.level = intel_panel_get_max_backlight(dev,
> +									  pipe);
>   		if (dev_priv->backlight.device)
>   			dev_priv->backlight.device->props.brightness =
>   				dev_priv->backlight.level;
> @@ -629,7 +648,8 @@ set_level:
>   	 * registers are set.
>   	 */
>   	dev_priv->backlight.enabled = true;
> -	intel_panel_actually_set_backlight(dev, dev_priv->backlight.level);
> +	intel_panel_actually_set_backlight(dev, pipe,
> +					   dev_priv->backlight.level);
>
>   	spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
>   }
> @@ -652,7 +672,7 @@ static void intel_panel_init_backlight(struct drm_device *dev)
>
>   	intel_panel_init_backlight_regs(dev);
>
> -	dev_priv->backlight.level = intel_panel_get_backlight(dev);
> +	dev_priv->backlight.level = intel_panel_get_backlight(dev, 0);
>   	dev_priv->backlight.enabled = dev_priv->backlight.level != 0;
>   }
>
> @@ -681,18 +701,31 @@ intel_panel_detect(struct drm_device *dev)
>   #if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>   static int intel_panel_update_status(struct backlight_device *bd)
>   {
> -	struct drm_device *dev = bl_get_data(bd);
> +	struct intel_connector *connector = bl_get_data(bd);
> +	struct drm_device *dev = connector->base.dev;
> +
> +	mutex_lock(&dev->mode_config.mutex);
>   	DRM_DEBUG_KMS("updating intel_backlight, brightness=%d/%d\n",
>   		      bd->props.brightness, bd->props.max_brightness);
> -	intel_panel_set_backlight(dev, bd->props.brightness,
> +	intel_panel_set_backlight(connector, bd->props.brightness,
>   				  bd->props.max_brightness);
> +	mutex_unlock(&dev->mode_config.mutex);
>   	return 0;
>   }
>
>   static int intel_panel_get_brightness(struct backlight_device *bd)
>   {
> -	struct drm_device *dev = bl_get_data(bd);
> -	return intel_panel_get_backlight(dev);
> +	struct intel_connector *connector = bl_get_data(bd);
> +	struct drm_device *dev = connector->base.dev;
> +	enum pipe pipe;
> +
> +	mutex_lock(&dev->mode_config.mutex);
> +	pipe = intel_get_pipe_from_connector(connector);
> +	mutex_unlock(&dev->mode_config.mutex);
> +	if (pipe == INVALID_PIPE)
> +		return 0;
> +
> +	return intel_panel_get_backlight(connector->base.dev, pipe);
>   }
>
>   static const struct backlight_ops intel_panel_bl_ops = {
> @@ -717,7 +750,7 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
>   	props.brightness = dev_priv->backlight.level;
>
>   	spin_lock_irqsave(&dev_priv->backlight.lock, flags);
> -	props.max_brightness = intel_panel_get_max_backlight(dev);
> +	props.max_brightness = intel_panel_get_max_backlight(dev, 0);
>   	spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
>
>   	if (props.max_brightness == 0) {
> @@ -726,7 +759,8 @@ int intel_panel_setup_backlight(struct drm_connector *connector)
>   	}
>   	dev_priv->backlight.device =
>   		backlight_device_register("intel_backlight",
> -					  connector->kdev, dev,
> +					  connector->kdev,
> +					  to_intel_connector(connector),
>   					  &intel_panel_bl_ops, &props);
>
>   	if (IS_ERR(dev_priv->backlight.device)) {
>

Tested-by: Joe Konno <joe.konno at intel.com>



More information about the Intel-gfx mailing list