From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755841AbdBNVm7 (ORCPT ); Tue, 14 Feb 2017 16:42:59 -0500 Received: from mail-it0-f66.google.com ([209.85.214.66]:35643 "EHLO mail-it0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752769AbdBNVm5 (ORCPT ); Tue, 14 Feb 2017 16:42:57 -0500 Date: Tue, 14 Feb 2017 22:42:53 +0100 From: Daniel Vetter To: John Stultz Cc: lkml , Chen Feng , Xinwei Kong , Xinliang Liu , dri-devel , Rongrong Zou , Daniel Vetter , Daniel Vetter Subject: Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs Message-ID: <20170214214253.GE11138@dvetter-linux.ger.corp.intel.com> Mail-Followup-To: John Stultz , lkml , Chen Feng , Xinwei Kong , Xinliang Liu , dri-devel , Rongrong Zou , Daniel Vetter References: <1487100302-9445-1-git-send-email-john.stultz@linaro.org> <1487100302-9445-2-git-send-email-john.stultz@linaro.org> <20170214202229.GG22762@dvetter-linux.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux dvetter-linux.ger.corp.intel.com 4.8.15-200.fc24.x86_64 User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 14, 2017 at 01:03:27PM -0800, John Stultz wrote: > On Tue, Feb 14, 2017 at 12:22 PM, Daniel Vetter wrote: > > On Tue, Feb 14, 2017 at 11:45:54AM -0800, John Stultz wrote: > >> > >> Not following here. What do you mean by "put it into drivers"? Where? > > > > In your driver callback for ->mode_valid, call into a shared function to > > validate CRTC limits. Which you also call from the crtc's ->mode_fixup > > function. > > So bascially have the adv7511 driver's mode_valid() have a special > callback to the kirin (and freedreno, and whatever other) drm driver > to check the modes? Or I guess the drm driver that uses that bridge > should register something w/ the adv7511 code? > > Part of my confusion here is that the main issue is that its not just > one driver I'm dealing with, and they're all really just tied together > via device tree, so I'm not sure how to special case it in just one of > the drivers. This sounds you want to fix this for bridges, but your patch only adds a crtc callback? > > In short my complain here is that this is only a partial solution of the > > larger problem, specific for your driver. That kind of code is better put > > into drivers, until we have a clear understanding to type up something > > complete in the helpers. Shared code is imo overrated :-) > > Yea, apologies for my not seeing the larger problem at first (its > still a bit hazy, really), part of this submission is just trying to > get something to throw darts at and figure out the right thing. > > But I'll try to figure out another approach here. Latest kerneldoc in drm-tip should explain this, not sure you didn't spot it or looked at an old kernel version. For drm docs, _always_ look at drm-tip, they're moving real fast :-) https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#modeset-helper-reference-for-common-vtables See e.g. drm_crtc_helpers_funcs->mode_fixup docs, it's there. There's explanations sprinkled at various places (mode_valid, plus the different helper funcs), probably good to first hunt these all down. Then read a bunch of driver callbacks so you have a better idea of the patterns of checks, otoh _lots_ of kms drivers get this wrong :( Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch