From patchwork Sun Feb 23 15:50:59 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Josh Boyer X-Patchwork-Id: 3705281 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4C4EABF13A for ; Sun, 23 Feb 2014 23:23:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3E23D201E4 for ; Sun, 23 Feb 2014 23:23:31 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 06232201DD for ; Sun, 23 Feb 2014 23:23:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2791FFA85D; Sun, 23 Feb 2014 15:23:24 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ie0-f170.google.com (mail-ie0-f170.google.com [209.85.223.170]) by gabe.freedesktop.org (Postfix) with ESMTP id DC8B8F9D8A; Sun, 23 Feb 2014 07:51:25 -0800 (PST) Received: by mail-ie0-f170.google.com with SMTP id rl12so2723324iec.15 for ; Sun, 23 Feb 2014 07:51:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=cO4vBe8nA7XY7SGhQPYwoluZpRSpWGrRtklQ9DPIvzM=; b=A6UYnMNbWMH4LjluZOWefZdn+JOv1nVCew+bYQ+GkplFe2yy1zx/deHzST13GhYkCu HwEK+Qx2d9W/OKGiyGwbGCCov59KJMvxWpglmM/HJRq67oZf5x8kiu4z3VEFHrvweUNP mKCiU6skqyGMJrIIZjTpMKFKDN+CnGb4qcwhfFGKO+deheaujrwlyOvxS0p8Gfz7OLTb xXw3IrwJ9y3Z7UoVMI6SDylBSaOcvqxoZBggQlmuh0JCFcjC1DkRdRh16iS7IgK7DjKM rkGoNbNDsnsEfp2qb7JQhw0WbnTrGjSehCL9Q0eO5gNbUUAKEUslApKT0zHpmZSIw2Mr w4cQ== X-Received: by 10.50.25.104 with SMTP id b8mr9547147igg.35.1393170685122; Sun, 23 Feb 2014 07:51:25 -0800 (PST) Received: from zod (75-128-125-27.dhcp.aldl.mi.charter.com. [75.128.125.27]) by mx.google.com with ESMTPSA id iq9sm16622982igb.7.2014.02.23.07.51.23 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 23 Feb 2014 07:51:24 -0800 (PST) Date: Sun, 23 Feb 2014 10:50:59 -0500 From: Josh Boyer To: Jesse Barnes , Jani Nikula Subject: Re: 3.13 i915 brightness settings broken when going from docked -> undocked Message-ID: <20140223155059.GA11167@zod> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Mailman-Approved-At: Sun, 23 Feb 2014 15:23:22 -0800 Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, "Linux-Kernel@Vger. Kernel. Org" , DRI mailing list X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Feb 20, 2014 at 07:31:41PM -0500, Josh Boyer wrote: >On Wed, Feb 19, 2014 at 9:20 PM, Josh Boyer wrote: >> Hi All, >> >> We've had a rather weird report[1] of the brightness adjustments being >> broken in a specific case with Thinkpad x220 hardware (SandyBridge >> based). If you boot the machine with it in a dock and then undock, >> the brightness adjustments do not work. That is with either the FN >> keys or the GNOME brightness slider. >> >> I can see that the value of >> /sys/class/backlight/acpi_video0/brightness increases/decreases but >> /sys/class/backlight/intel_backlight/brightness doesn't reflect any >> changes. With 3.12 this works, and oddly with 3.14-rc1 it works >> (specifically, it starts working around v3.13-10231-g53d8ab2 which is >> right after the first DRM merge for 3.14). With 3.13, if I undock and >> echo a higher value in the intel_backlight_brightness sysfs entry, the >> brightness will actually increase so it can be done manually, but it >> does not work as you'd expect. >> >> I'm in the middle of trying to do a reverse bisect for which patch >> fixes it in the 3.14-rcX series, but that's taking a while. I thought >> I'd email and see if anyone already knows about this situation, what >> patch in 3.13 broke this, and which one then fixed it again. Thus far >> all I've gathered is that backlight handling is confusing. > >The reverse bisect between 3.13 and 3.14-rc1 didn't prove fruitful, >either because I messed it up or there's a combination of things that >fix the issue. So instead I did a regular git bisect between 3.12 and >3.13 to see which commit _broke_ things and caused the above behavior. > That landed me at: > >Author: Jesse Barnes >Date: Thu Oct 31 18:55:49 2013 +0200 > > drm/i915: make backlight functions take a connector > >I have no idea if that makes sense or not, but it's at least something >that seems to be in a relevant area of code. Does anyone involved in >that know why it would cause the above symptoms on 3.13, and which >commit(s) fix it in 3.14-rc1? Since nobody is replying I poked around a bit myself. The one commit that looks somewhat relevant in 3.14-rc1 seems to be: commit c91c9f32843a1b433de5a1ead4789a6bc8d3d914 Author: Jani Nikula Date: Fri Nov 8 16:48:55 2013 +0200 drm/i915: make asle notifications update backlight on all connectors That doesn't apply cleanly on 3.13 because of the other reworks that went in first, so I came up with the patch below. It seems to fix it for my machine, but I'm waiting for confirmation from the original bug reporter still. Maybe this will elicit some comments? josh Backport of upstream commit c91c9f328 --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_opregion.c | 31 ++++++------------------------- drivers/gpu/drm/i915/intel_panel.c | 4 ++++ 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 221ac62..d6d4349 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1371,6 +1371,7 @@ typedef struct drm_i915_private { /* backlight */ struct { + bool present; int level; bool enabled; spinlock_t lock; /* bl registers and the above bl fields */ diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index 6d69a9b..b2a51ae 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -414,38 +414,19 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 bclp) 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); + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { + intel_connector = to_intel_connector(connector); + if (dev_priv->backlight.present) + intel_panel_set_backlight(intel_connector, bclp, 255); } - DRM_DEBUG_KMS("updating opregion backlight %d/255\n", bclp); - intel_panel_set_backlight(intel_connector, bclp, 255); iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv); -out: mutex_unlock(&dev->mode_config.mutex); - return ret; + return 0; } 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 e6f782d..fa7b984 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -832,6 +832,9 @@ int intel_panel_setup_backlight(struct drm_connector *connector) dev_priv->backlight.device = NULL; return -ENODEV; } + + dev_priv->backlight.present = true; + return 0; } @@ -839,6 +842,7 @@ void intel_panel_destroy_backlight(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; if (dev_priv->backlight.device) { + dev_priv->backlight.present = false; backlight_device_unregister(dev_priv->backlight.device); dev_priv->backlight.device = NULL; }