Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933688AbdGKPoU (ORCPT ); Tue, 11 Jul 2017 11:44:20 -0400 Received: from mail-qk0-f172.google.com ([209.85.220.172]:34222 "EHLO mail-qk0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933419AbdGKPoS (ORCPT ); Tue, 11 Jul 2017 11:44:18 -0400 MIME-Version: 1.0 In-Reply-To: References: <1499719682-31750-1-git-send-email-john.stultz@linaro.org> <20170710211833.bprcrq5tfpzxtpeg@art_vandelay> <20170711075625.peykmamxkt7slqdz@phenom.ffwll.local> From: John Stultz Date: Tue, 11 Jul 2017 08:44:16 -0700 Message-ID: Subject: Re: [RFC][PATCH] drm: kirin: Restrict modes to known good mode clocks To: Daniel Vetter Cc: Sean Paul , Jose Abreu , Chen Feng , lkml , Xinliang Liu , "dri-devel@lists.freedesktop.org" , Rongrong Zou , Daniel Vetter , Xinwei Kong Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2319 Lines: 47 On Tue, Jul 11, 2017 at 8:12 AM, Daniel Vetter wrote: > On Tue, Jul 11, 2017 at 5:05 PM, John Stultz wrote: >>>> > be even better if you could calculate whether the mode is valid, but I didn't >>>> > spend enough time to figure out if this is possible. >>>> >>>> Theoretically that might be possible, checking if the requested freq >>>> matches the calculated freq, and I've tried that but so far I've not >>>> been able to get it to work, as in some cases the freq on the current >>>> whitelist don't exactly match but do work on the large majority of >>>> monitors tested (while other freq have a similar error but don't work >>>> on most monitors). >>>> >>>> I'd like to spend some more time to try to refine and tune this, but >>>> having the current whitelist works fairly well, so I'm not sure its >>>> worth sitting on (this is basically the last functional patch >>>> outstanding for HiKey to fully work upstream - except the mali gpu of >>>> course), while I try to tinker and tune it. >>>> >>>> Thanks so much for the feedback! >>> >>> Yeah the proper approach is to compute your pll/clock settings and bail >>> out if those don't work. That's generally a magic spreadsheet supplied by >>> the hw validation engineers, and I honestly don't want to know how they >>> create it. Explicit modelist in the kernel sounds like a very bad hack. >> >> So without such a magic spreadsheet, how would you suggest I move this forward? >> Not having the whitelist hack and picking modes the device can't >> generate is a fairly major usability issue. > > I guess if the whitelist is the only thing I'd do 2 things differently: > - Whitelist the clocks, not modes, since that's what seems to matter here. > - Put it as close as possible to the code that comuptes the clock > settings (yet if you use the clock subsystem that's a bit hard, but > for an atomic driver this should be where this is done ...). > > Whitelist of modes looks super-hacky. Sure. The whitelist modes were easiest to use initially dealing with problem reports since the EDID numbers were what users could report working or not. But this feedback sounds reasonable, as I can map those to the underlying pixel clocks and generate a whitelist of those. I really appreciate the feedback here! thanks -john