From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Konno Subject: Re: [PATCH 2/7] drm/i915: make backlight functions take a connector Date: Wed, 06 Nov 2013 09:08:40 -0800 Message-ID: <527A7798.1020704@linux.intel.com> References: <49dbff618fc224ba16ba7235d4fa8e3a8308c247.1383237868.git.jani.nikula@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 01CC8F9F52 for ; Wed, 6 Nov 2013 09:09:16 -0800 (PST) In-Reply-To: <49dbff618fc224ba16ba7235d4fa8e3a8308c247.1383237868.git.jani.nikula@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 10/31/2013 09:55 AM, Jani Nikula wrote: > From: Jesse Barnes > > 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 > Signed-off-by: Jani Nikula > --- > 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@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