From: Joe Konno <joe.konno@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/7] drm/i915: make backlight info per-connector
Date: Wed, 06 Nov 2013 09:12:15 -0800 [thread overview]
Message-ID: <527A786F.2080607@linux.intel.com> (raw)
In-Reply-To: <ffd1a0890a172de0edf8135815ebc97f5d733b34.1383237868.git.jani.nikula@intel.com>
On 10/31/2013 09:55 AM, Jani Nikula wrote:
> Move from dev_priv to connector->panel.
>
> We still don't allow multiple sysfs interfaces, though.
>
> There should be no functional changes.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 9 +---
> drivers/gpu/drm/i915/i915_suspend.c | 8 ++--
> drivers/gpu/drm/i915/intel_drv.h | 7 +++
> drivers/gpu/drm/i915/intel_panel.c | 85 +++++++++++++++++++----------------
> 5 files changed, 61 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 0cab2d0..9a2a175 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1486,7 +1486,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>
> spin_lock_init(&dev_priv->irq_lock);
> spin_lock_init(&dev_priv->gpu_error.lock);
> - spin_lock_init(&dev_priv->backlight.lock);
> + spin_lock_init(&dev_priv->backlight_lock);
> spin_lock_init(&dev_priv->uncore.lock);
> spin_lock_init(&dev_priv->mm.object_stat_lock);
> mutex_init(&dev_priv->dpio_lock);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 16fae30..16b93a6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1349,13 +1349,8 @@ typedef struct drm_i915_private {
> struct intel_overlay *overlay;
> unsigned int sprite_scaling_enabled;
>
> - /* backlight */
> - struct {
> - int level;
> - bool enabled;
> - spinlock_t lock; /* bl registers and the above bl fields */
> - struct backlight_device *device;
> - } backlight;
> + /* backlight registers and fields in struct intel_panel */
> + spinlock_t backlight_lock;
>
> /* LVDS info */
> bool no_aux_handshake;
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> index 98790c7..eadf8e1 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -203,7 +203,7 @@ static void i915_save_display(struct drm_device *dev)
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> i915_save_display_reg(dev);
>
> - spin_lock_irqsave(&dev_priv->backlight.lock, flags);
> + spin_lock_irqsave(&dev_priv->backlight_lock, flags);
>
> /* LVDS state */
> if (HAS_PCH_SPLIT(dev)) {
> @@ -241,7 +241,7 @@ static void i915_save_display(struct drm_device *dev)
> dev_priv->regfile.saveLVDS = I915_READ(LVDS);
> }
>
> - spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
> + spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
>
> if (!IS_I830(dev) && !IS_845G(dev) && !HAS_PCH_SPLIT(dev))
> dev_priv->regfile.savePFIT_CONTROL = I915_READ(PFIT_CONTROL);
> @@ -287,7 +287,7 @@ static void i915_restore_display(struct drm_device *dev)
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> i915_restore_display_reg(dev);
>
> - spin_lock_irqsave(&dev_priv->backlight.lock, flags);
> + spin_lock_irqsave(&dev_priv->backlight_lock, flags);
>
> /* LVDS state */
> if (INTEL_INFO(dev)->gen >= 4 && !HAS_PCH_SPLIT(dev))
> @@ -341,7 +341,7 @@ static void i915_restore_display(struct drm_device *dev)
> I915_WRITE(PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
> }
>
> - spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
> + spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
>
> /* only restore FBC info on the platform that supports FBC*/
> intel_disable_fbc(dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5548180..9460e54 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -156,6 +156,13 @@ struct intel_encoder {
> struct intel_panel {
> struct drm_display_mode *fixed_mode;
> int fitting_mode;
> +
> + /* backlight */
> + struct {
> + u32 level;
> + bool enabled;
> + struct backlight_device *device;
> + } backlight;
> };
>
> struct intel_connector {
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index a0d13d3..0a4aeaf 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -346,7 +346,7 @@ 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;
>
> - WARN_ON_SMP(!spin_is_locked(&dev_priv->backlight.lock));
> + WARN_ON_SMP(!spin_is_locked(&dev_priv->backlight_lock));
>
> /* Restore the CTL value if it lost, e.g. GPU reset */
>
> @@ -449,7 +449,7 @@ static u32 intel_panel_get_backlight(struct drm_device *dev,
> unsigned long flags;
> int reg;
>
> - spin_lock_irqsave(&dev_priv->backlight.lock, flags);
> + spin_lock_irqsave(&dev_priv->backlight_lock, flags);
>
> if (HAS_PCH_SPLIT(dev)) {
> val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
> @@ -473,7 +473,7 @@ static u32 intel_panel_get_backlight(struct drm_device *dev,
>
> val = intel_panel_compute_brightness(dev, pipe, val);
>
> - spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
> + spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
>
> DRM_DEBUG_DRIVER("get backlight PWM = %d\n", val);
> return val;
> @@ -530,6 +530,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_panel *panel = &connector->panel;
> enum pipe pipe = intel_get_pipe_from_connector(connector);
> u32 freq;
> unsigned long flags;
> @@ -537,7 +538,7 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
> if (pipe == INVALID_PIPE)
> return;
>
> - spin_lock_irqsave(&dev_priv->backlight.lock, flags);
> + spin_lock_irqsave(&dev_priv->backlight_lock, flags);
>
> freq = intel_panel_get_max_backlight(dev, pipe);
> if (!freq) {
> @@ -551,20 +552,21 @@ void intel_panel_set_backlight(struct intel_connector *connector, u32 level,
> else
> level = freq / max * level;
>
> - dev_priv->backlight.level = level;
> - if (dev_priv->backlight.device)
> - dev_priv->backlight.device->props.brightness = level;
> + panel->backlight.level = level;
> + if (panel->backlight.device)
> + panel->backlight.device->props.brightness = level;
>
> - if (dev_priv->backlight.enabled)
> + if (panel->backlight.enabled)
> intel_panel_actually_set_backlight(dev, pipe, level);
> out:
> - spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
> + spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
> }
>
> 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;
> + struct intel_panel *panel = &connector->panel;
> enum pipe pipe = intel_get_pipe_from_connector(connector);
> unsigned long flags;
>
> @@ -582,9 +584,9 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
> return;
> }
>
> - spin_lock_irqsave(&dev_priv->backlight.lock, flags);
> + spin_lock_irqsave(&dev_priv->backlight_lock, flags);
>
> - dev_priv->backlight.enabled = false;
> + panel->backlight.enabled = false;
> intel_panel_actually_set_backlight(dev, pipe, 0);
>
> if (INTEL_INFO(dev)->gen >= 4) {
> @@ -606,13 +608,14 @@ void intel_panel_disable_backlight(struct intel_connector *connector)
> }
> }
>
> - spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
> + spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
> }
>
> 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;
> + struct intel_panel *panel = &connector->panel;
> enum pipe pipe = intel_get_pipe_from_connector(connector);
> enum transcoder cpu_transcoder =
> intel_pipe_to_cpu_transcoder(dev_priv, pipe);
> @@ -623,14 +626,14 @@ void intel_panel_enable_backlight(struct intel_connector *connector)
>
> DRM_DEBUG_KMS("pipe %c\n", pipe_name(pipe));
>
> - spin_lock_irqsave(&dev_priv->backlight.lock, flags);
> + spin_lock_irqsave(&dev_priv->backlight_lock, flags);
>
> - if (dev_priv->backlight.level == 0) {
> - 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;
> + if (panel->backlight.level == 0) {
> + panel->backlight.level = intel_panel_get_max_backlight(dev,
> + pipe);
> + if (panel->backlight.device)
> + panel->backlight.device->props.brightness =
> + panel->backlight.level;
> }
>
> if (INTEL_INFO(dev)->gen >= 4) {
> @@ -680,11 +683,11 @@ set_level:
> * BLC_PWM_CPU_CTL may be cleared to zero automatically when these
> * registers are set.
> */
> - dev_priv->backlight.enabled = true;
> + panel->backlight.enabled = true;
> intel_panel_actually_set_backlight(dev, pipe,
> - dev_priv->backlight.level);
> + panel->backlight.level);
>
> - spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
> + spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
> }
>
> /* FIXME: use VBT vals to init PWM_CTL and PWM_CTL2 correctly */
> @@ -770,34 +773,40 @@ static int intel_backlight_device_register(struct intel_connector *connector)
> {
> struct drm_device *dev = connector->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_panel *panel = &connector->panel;
> struct backlight_properties props;
> unsigned long flags;
>
> - if (WARN_ON(dev_priv->backlight.device))
> + if (WARN_ON(panel->backlight.device))
> return -ENODEV;
>
> memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_RAW;
> - props.brightness = dev_priv->backlight.level;
> + props.brightness = panel->backlight.level;
>
> - spin_lock_irqsave(&dev_priv->backlight.lock, flags);
> + spin_lock_irqsave(&dev_priv->backlight_lock, flags);
> props.max_brightness = intel_panel_get_max_backlight(dev, 0);
> - spin_unlock_irqrestore(&dev_priv->backlight.lock, flags);
> + spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
>
> if (props.max_brightness == 0) {
> DRM_DEBUG_DRIVER("Failed to get maximum backlight value\n");
> return -ENODEV;
> }
> - dev_priv->backlight.device =
> +
> + /*
> + * Note: using the same name independent of the connector prevents
> + * registration of multiple backlight devices in the driver.
> + */
> + panel->backlight.device =
> backlight_device_register("intel_backlight",
> connector->base.kdev,
> connector,
> &intel_backlight_device_ops, &props);
>
> - if (IS_ERR(dev_priv->backlight.device)) {
> + if (IS_ERR(panel->backlight.device)) {
> DRM_ERROR("Failed to register backlight: %ld\n",
> - PTR_ERR(dev_priv->backlight.device));
> - dev_priv->backlight.device = NULL;
> + PTR_ERR(panel->backlight.device));
> + panel->backlight.device = NULL;
> return -ENODEV;
> }
> return 0;
> @@ -805,11 +814,11 @@ static int intel_backlight_device_register(struct intel_connector *connector)
>
> static void intel_backlight_device_unregister(struct intel_connector *connector)
> {
> - struct drm_device *dev = connector->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - if (dev_priv->backlight.device) {
> - backlight_device_unregister(dev_priv->backlight.device);
> - dev_priv->backlight.device = NULL;
> + struct intel_panel *panel = &connector->panel;
> +
> + if (panel->backlight.device) {
> + backlight_device_unregister(panel->backlight.device);
> + panel->backlight.device = NULL;
> }
> }
> #else /* CONFIG_BACKLIGHT_CLASS_DEVICE */
> @@ -825,13 +834,13 @@ static void intel_backlight_device_unregister(struct intel_connector *connector)
> int intel_panel_setup_backlight(struct drm_connector *connector)
> {
> struct drm_device *dev = connector->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_connector *intel_connector = to_intel_connector(connector);
> + struct intel_panel *panel = &intel_connector->panel;
>
> intel_panel_init_backlight_regs(dev);
>
> - dev_priv->backlight.level = intel_panel_get_backlight(dev, 0);
> - dev_priv->backlight.enabled = dev_priv->backlight.level != 0;
> + panel->backlight.level = intel_panel_get_backlight(dev, 0);
> + panel->backlight.enabled = panel->backlight.level != 0;
>
> intel_backlight_device_register(intel_connector);
>
>
Tested-by: Joe Konno <joe.konno@intel.com>
next prev parent reply other threads:[~2013-11-06 17:12 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-31 16:55 [PATCH 0/7] drm/i915: per connector backlight, per chip vfuncs Jani Nikula
2013-10-31 16:55 ` [PATCH 1/7] drm/i915: move opregion asle request handling to a work queue Jani Nikula
2013-11-05 17:50 ` Jesse Barnes
2013-11-06 17:07 ` Joe Konno
2013-10-31 16:55 ` [PATCH 2/7] drm/i915: make backlight functions take a connector Jani Nikula
2013-11-06 17:08 ` Joe Konno
2013-10-31 16:55 ` [PATCH 3/7] drm/i915/vlv: use per-pipe backlight controls v2 Jani Nikula
2013-11-06 17:09 ` Joe Konno
2013-10-31 16:55 ` [PATCH 4/7] drm/i915: clean up backlight conditional build Jani Nikula
2013-11-06 17:11 ` Joe Konno
2013-10-31 16:55 ` [PATCH 5/7] drm/i915: make backlight info per-connector Jani Nikula
2013-11-06 17:12 ` Joe Konno [this message]
2013-10-31 16:55 ` [PATCH 6/7] drm/i915: handle backlight through chip specific functions Jani Nikula
2013-11-06 17:12 ` Joe Konno
2013-10-31 16:55 ` [PATCH 7/7] drm/i915: make asle notifications update backlight on all connectors Jani Nikula
2013-11-06 17:12 ` Joe Konno
2013-10-31 21:34 ` [PATCH 0/7] drm/i915: per connector backlight, per chip vfuncs Joe Konno
2013-11-01 13:28 ` Jani Nikula
2013-11-01 14:30 ` Joe Konno
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=527A786F.2080607@linux.intel.com \
--to=joe.konno@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/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.