From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sn1nam02on0088.outbound.protection.outlook.com ([104.47.36.88]:5120 "EHLO NAM02-SN1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726617AbeJIUdt (ORCPT ); Tue, 9 Oct 2018 16:33:49 -0400 Subject: Re: [PATCH 4.18 222/235] drm/amd/pp: Send khz clock values to DC for smu7/8 To: "Deucher, Alexander" , Sasha Levin Cc: Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" , "Zhu, Rex" , Sasha Levin References: <20180924113103.999624566@linuxfoundation.org> <20180924113125.488862921@linuxfoundation.org> <20181008144917.GE32006@sasha-vm> From: Harry Wentland Message-ID: <86281b7c-8940-5d1b-e164-187087519dc8@amd.com> Date: Tue, 9 Oct 2018 09:16:42 -0400 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: On 2018-10-08 12:17 PM, Deucher, Alexander wrote: >> -----Original Message----- >> From: Sasha Levin >> Sent: Monday, October 8, 2018 10:49 AM >> To: Deucher, Alexander >> Cc: Greg Kroah-Hartman ; linux- >> kernel@vger.kernel.org; stable@vger.kernel.org; Wentland, Harry >> ; Zhu, Rex ; Sasha Levin >> >> Subject: Re: [PATCH 4.18 222/235] drm/amd/pp: Send khz clock values to DC >> for smu7/8 >> >> On Mon, Oct 08, 2018 at 02:33:56PM +0000, Deucher, Alexander wrote: >>>> -----Original Message----- >>>> From: Greg Kroah-Hartman >>>> Sent: Monday, September 24, 2018 7:53 AM >>>> To: linux-kernel@vger.kernel.org >>>> Cc: Greg Kroah-Hartman ; >>>> stable@vger.kernel.org; Wentland, Harry ; >>>> Deucher, Alexander ; Zhu, Rex >>>> ; Sasha Levin >>>> Subject: [PATCH 4.18 222/235] drm/amd/pp: Send khz clock values to DC >>>> for >>>> smu7/8 >>>> >>>> 4.18-stable review patch. If anyone has any objections, please let me >> know. >>>> >>> >>> This regresses power usage on 4.18. Please revert. >>> https://bugzilla.kernel.org/show_bug.cgi?id=201275 >> >> Hi Alex, >> >> Thank you for the report. >> >> I'm working on improving this process, I'd be very grateful if you could >> answer a few questions about this: >> >> 1. Is the same breakage seen upstream? (if so, it should be reverted there as >> well and we can grab the revert into -stable). > > No regression in 4.19 or -next. > >> 2. Does the issue reported by this patch ("pipes seem to hang with a 4k DP >> and 1080p HDMI display") exist in the 4.18 stable tree? > > I don't think so, but I'm not 100% sure. Harry, Rex do you know if this is a general issue or was it just fall out from the changes to the interface? > It was intended to deal with fallout to smu7/8 caused by the change to move the kHz-to-10kHz-conversion from display to powerplay. I believe that conversion initially overlooked these blocks. Rex, please correct me if I'm wrong. You probably know more about this than I do. So yes, I agree, it was never intended to be used without the change to drop the *10 in display code, which was done with this change: commit 23ec3d1479fd79658cd52c47618d8ddd2f32550b Author: Rex Zhu Date: Mon Jun 18 18:15:15 2018 +0800 drm/amd/pp: Convert clock unit to KHz as defined Convert clock unit 10KHz to KHz as the data sturct defined. e.g. struct pp_clock_with_latency { uint32_t clocks_in_khz; uint32_t latency_in_us; }; Meanwhile revert the same conversion in display side. Acked-by: Alex Deucher Acked-by: Harry Wentland Signed-off-by: Rex Zhu Signed-off-by: Alex Deucher I don't see this change in the 4.18.y stable tree. I probably should've caught that with the initial 4.18-stable review patch. Harry >> 3. If not, could you briefly explain why? > > We refactored the interface between the power and display components and this patch fixed up some of that fallout due to the differences in units used in each component. > >> >> >> The algorithm I use was very confident about this patch being stable material, >> and when I looked at it back then (and again now) I was very confident of the >> same. If I can understand where I was wrong I could improve my process. > > There are some other dependent patches required that were not flagged in the patch itself. IIRC, they were a bit big for stable. > > Alex >