From patchwork Sun Jan 13 21:00:03 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eldad Zack X-Patchwork-Id: 1971201 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 45032DF23A for ; Mon, 14 Jan 2013 07:30:35 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 655FAE5EA7 for ; Sun, 13 Jan 2013 23:30:35 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-bk0-f42.google.com (mail-bk0-f42.google.com [209.85.214.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 4D8B1E5F87 for ; Sun, 13 Jan 2013 13:00:14 -0800 (PST) Received: by mail-bk0-f42.google.com with SMTP id ji2so1648569bkc.29 for ; Sun, 13 Jan 2013 13:00:13 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:date:from:x-x-sender:to:cc:subject:in-reply-to :message-id:references:user-agent:mime-version:content-type :x-gm-message-state; bh=kKY0dL/4IVJV7KowH2rAksNN46WpjoDziB6nFy/qi38=; b=Nn5R/LGfVfjptYsTmzUqGaKgIp/E5aFwRPJtW1HettUfWWCeO7EbhZAIZwFzL1fTmW 2exRRG9lseS5T2NWzQzZzmP8nizhAkwyOkcpl1gY8gwH94GZXjYjrmyyjqU3V+ZAtxkY lxlv6lIyUOHfxdt/MTlsl3TSugD8qlza1YZumhQH2c4344Sq6+hPbc1rhTgxVRdg86ub aUcHi6098SQnxRyX0qIcESk+2+gQNQu/He2Xeti2sYjNqf1gbaeQewZXtKgpFbacXtyZ KmuSggCbqA3od3xlOB5SnnBGgeSyfuzQssXU/FCDrvxQL0FqNi+DS50MIJPr7jthWvjt pFSQ== X-Received: by 10.204.156.81 with SMTP id v17mr38118597bkw.18.1358110813195; Sun, 13 Jan 2013 13:00:13 -0800 (PST) Received: from [192.168.2.10] (p5DDC6AB5.dip.t-dialin.net. [93.220.106.181]) by mx.google.com with ESMTPS id f24sm8328189bkv.7.2013.01.13.13.00.11 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Sun, 13 Jan 2013 13:00:12 -0800 (PST) Date: Sun, 13 Jan 2013 22:00:03 +0100 (CET) From: Eldad Zack X-X-Sender: eldad@xoschi To: Alex Deucher Subject: Re: Regression: drm/radeon: brightness control hard system lockup In-Reply-To: Message-ID: References: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 X-Gm-Message-State: ALoCoQlc1AXUwMUdpFdJtiBqzOpovBnq1aQhgWmj9Xg6dpxuKT5wYxrVGtFBXSCry4ckEgGeIza8 X-Mailman-Approved-At: Sun, 13 Jan 2013 23:30:18 -0800 Cc: Alex Deucher , Jerome Glisse , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org 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+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org On Mon, 7 Jan 2013, Alex Deucher wrote: > On Mon, Jan 7, 2013 at 4:33 PM, Eldad Zack wrote: > > > > On Mon, 7 Jan 2013, Alex Deucher wrote: > >> On Sun, Jan 6, 2013 at 7:59 AM, Eldad Zack wrote: > >> > > >> > Hi Alex, > >> > > >> > Commit 0ecebb9e0d14e9948e0b1529883a776758117d6f "drm/radeon: switch to a > >> > finer grained reset for evergreen" introduced a hard system lockup to my > >> > setup. I found it after bisecting, and confirmed it by reverting it on > >> > the latest mainline ( 5f243b9 ). > >> > > >> > This: > >> > > >> > echo 7 > /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/backlight/acpi_video0/brightness > >> > > >> > Causes a hard lock-up hard, i.e. immediate freeze, without any logs. > >> > > >> > See lspci output and kernel .config below. > >> > If there's any more info I can provide, please let me know. > >> > >> Do you normally see GPU resets when changing the backlight? Please > >> attach your dmesg output when changing the backlight with the patch > >> reverted. > > > > I see nothing. Just to make sure, I cleared the buffer, cycled through > > 0-7 a couple of hunderd times (until the flicker annoyed), but I see no > > messages at all. > > Is there any debug config I should turn on? > > Can you try adding a printk() in evergreen_asic_reset() and see if it > is somehow getting called when you change the brightness? When you > use the apci backlight control, the radeon driver is not involved at > all. They only way the driver would get involved is if the acpi > backlight control somehow caused the GPU to hang and then the driver > detected the hang and attempted to reset the GPU. I don't see any > evidence of a GPU reset in your kernel log however. Note that the > driver also registers native backlight contol. Does that work any > better than acpi? The native backlight controls work very well. Thanks for that, I didn't even noticed that. It has finer control over brightness too. I worked out a fix for the problem, but I think it's not a proper one. What I noticed is that evergreen_gpu_soft_reset() is only ever called once on my system, at boot. Then I realized from the dmesg that neither evergreen_gpu_soft_reset_dma() nor evergreen_gpu_soft_reset_gfx() actually do anything. Both return on the first if statements there. So as far as I can tell, the difference your patch introduced is calling evergreen_mc_stop() and evergreen_mc_resume(), which somehow puts my system in some state that ACPI brightness control leads to a lock up. BTW, I don't see a GPU reset happening at all - I also tried suspend/resume and starting an OpenGL application. Do you know how I can trigger it? To fix this, I moved the GUI_ACTIVE test before evergreen_mc_stop(), but I think it just masks the issue. Patch below (against latest HEAD) just in case. Cheers, Eldad From 5817128d2761f60051b069d2bb31209c909b6a04 Mon Sep 17 00:00:00 2001 From: Eldad Zack Date: Sun, 13 Jan 2013 21:14:08 +0100 Subject: [PATCH] drm/radeon: fix evergreen brightness control regression Commit 0ecebb9e0d14e9948e0b1529883a776758117d6f introduced a regression, where using ACPI brightness control leads to a hard system lock-up. To resolve the issue, this patch moves the GUI_ACTIVE test earlier, as it was before the commit. Signed-off-by: Eldad Zack --- drivers/gpu/drm/radeon/evergreen.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 061fa0a..1392f99 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -2310,9 +2310,6 @@ static void evergreen_gpu_soft_reset_gfx(struct radeon_device *rdev) { u32 grbm_reset = 0; - if (!(RREG32(GRBM_STATUS) & GUI_ACTIVE)) - return; - dev_info(rdev->dev, " GRBM_STATUS = 0x%08X\n", RREG32(GRBM_STATUS)); dev_info(rdev->dev, " GRBM_STATUS_SE0 = 0x%08X\n", @@ -2404,6 +2401,9 @@ static int evergreen_gpu_soft_reset(struct radeon_device *rdev, u32 reset_mask) if (reset_mask == 0) return 0; + if (!(RREG32(GRBM_STATUS) & GUI_ACTIVE)) + return 0; + dev_info(rdev->dev, "GPU softreset: 0x%08X\n", reset_mask); evergreen_mc_stop(rdev, &save);