From: Imre Deak <imre.deak@intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 02/13] drm/i915: make backlight info per-connector
Date: Tue, 12 Nov 2013 23:29:15 +0200 [thread overview]
Message-ID: <1384291755.2462.17.camel@ideak-mobl> (raw)
In-Reply-To: <7f2e4bdda2f55f7c3ecd8fe430a4e3f6b242bbb2.1383920621.git.jani.nikula@intel.com>
On Fri, 2013-11-08 at 16:48 +0200, 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, except for a slight reordering of
> connector backlight and sysfs destroy calls. (This change happens now
> that the backlight device is actually per-connector, even though the
> destroy calls became per-connector earlier.)
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Imre Deak <imre.deak@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 67117e7..0517d47 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1368,13 +1368,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 1625580..a0d8dda 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);
>
next prev parent reply other threads:[~2013-11-12 21:29 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-08 14:48 [PATCH 00/13] drm/i915: backlight rewrite Jani Nikula
2013-11-08 14:48 ` [PATCH 01/13] drm/i915: clean up backlight conditional build Jani Nikula
2013-11-12 21:23 ` Imre Deak
2013-11-08 14:48 ` [PATCH 02/13] drm/i915: make backlight info per-connector Jani Nikula
2013-11-12 21:29 ` Imre Deak [this message]
2013-11-08 14:48 ` [PATCH 03/13] drm/i915: make asle notifications update backlight on all connectors Jani Nikula
2013-11-12 21:29 ` Imre Deak
2013-11-08 14:48 ` [PATCH 04/13] drm/i915: handle backlight through chip specific functions Jani Nikula
2013-11-12 21:36 ` Imre Deak
2013-11-12 23:19 ` Daniel Vetter
2013-11-08 14:48 ` [PATCH 05/13] drm/i915: fix gen2-gen3 backlight set Jani Nikula
2013-11-12 22:00 ` Imre Deak
2013-11-13 8:27 ` Jani Nikula
2013-11-13 9:04 ` Daniel Vetter
2013-11-13 9:12 ` Imre Deak
2013-11-08 14:48 ` [PATCH 06/13] drm/i915: vlv does not have pipe field in backlight registers Jani Nikula
2013-11-12 22:00 ` Imre Deak
2013-11-08 14:48 ` [PATCH 07/13] drm/i915: move backlight level setting in enable/disable to hooks Jani Nikula
2013-11-12 22:01 ` Imre Deak
2013-11-08 14:49 ` [PATCH 08/13] drm/i915: use the initialized backlight max value instead of reading it Jani Nikula
2013-11-12 22:42 ` Imre Deak
2013-11-13 8:39 ` Jani Nikula
2013-11-13 9:12 ` Daniel Vetter
2013-11-08 14:49 ` [PATCH 09/13] drm/i915: debug print on backlight register Jani Nikula
2013-11-12 22:48 ` Imre Deak
2013-11-13 10:22 ` Daniel Vetter
2013-11-08 14:49 ` [PATCH 10/13] drm/i915: gather backlight information at setup Jani Nikula
2013-11-13 17:01 ` Imre Deak
2013-11-14 5:19 ` Jani Nikula
2013-11-14 8:22 ` Imre Deak
2013-11-08 14:49 ` [PATCH 11/13] drm/i915: do full backlight setup at enable time Jani Nikula
2013-11-13 17:53 ` Imre Deak
2013-11-14 5:43 ` Jani Nikula
2013-11-14 8:27 ` Daniel Vetter
2013-11-14 8:28 ` Imre Deak
2013-11-14 10:13 ` [PATCH v2 " Jani Nikula
2013-11-14 10:46 ` Imre Deak
2013-11-14 10:14 ` [PATCH 11.5/13] drm/i915: remove QUIRK_NO_PCH_PWM_ENABLE Jani Nikula
2013-11-14 10:50 ` Imre Deak
2013-11-08 14:49 ` [PATCH 12/13] drm/i915: nuke get max backlight functions Jani Nikula
2013-11-13 17:54 ` Imre Deak
2013-11-08 14:49 ` [PATCH 13/13] drm/i915: do not save/restore backlight registers Jani Nikula
2013-11-12 23:25 ` Daniel Vetter
2013-11-13 8:40 ` Jani Nikula
2013-11-13 10:56 ` [PATCH v2] drm/i915: do not save/restore backlight registers in KMS Jani Nikula
2013-11-13 18:05 ` Imre Deak
2013-11-14 11:22 ` Daniel Vetter
2013-11-11 8:36 ` [PATCH 00/13] drm/i915: backlight rewrite Jani Nikula
2013-11-12 21:22 ` Imre Deak
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=1384291755.2462.17.camel@ideak-mobl \
--to=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox