From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs Date: Tue, 14 Feb 2017 21:51:29 +0200 Message-ID: <20170214195129.GN31595@intel.com> References: <1487100302-9445-1-git-send-email-john.stultz@linaro.org> <1487100302-9445-2-git-send-email-john.stultz@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id A895D6E7F5 for ; Tue, 14 Feb 2017 19:51:34 +0000 (UTC) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Chen Feng , lkml , dri-devel , Xinliang Liu , Xinwei Kong , Rongrong Zou , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBGZWIgMTQsIDIwMTcgYXQgMDg6Mzg6NDBQTSArMDEwMCwgRGFuaWVsIFZldHRlciB3 cm90ZToKPiBPbiBUdWUsIEZlYiAxNCwgMjAxNyBhdCA4OjI1IFBNLCBKb2huIFN0dWx0eiA8am9o bi5zdHVsdHpAbGluYXJvLm9yZz4gd3JvdGU6Cj4gPiBDdXJyZW50bHksIG9uIHRoZSBoaWtleSBi b2FyZCwgd2UgaGF2ZSB0aGUgYWR2NzUxMSBicmlkZ2Ugd2lyZWQKPiA+IHVwIHRvIHRoZSBraXJp biBhZGUgZHJtIGRyaXZlci4gVW5mb3J0dW5hdGVseSwgdGhlIGtpcmluIGFkZQo+ID4gY29yZSBj YW5ub3QgZ2VuZXJhdGUgYWNjdXJhdGUgYnl0ZWNsb2NrcyBmb3IgYWxsIHBpeGVsIGNsb2NrCj4g PiB2YWx1ZXMuCj4gPgo+ID4gVGh1cyBpZiBhIG1vZGUgY2xvY2sgaXMgc2VsZWN0ZWQgdGhhdCB3 ZSBjYW5ub3QgY2FsY3VsYXRlIGEKPiA+IG1hdGNoaW5nIGJ5dGVjbG9jaywgdGhlIGRldmljZSB3 aWxsIGJvb3Qgd2l0aCBhIGJsYW5rIHNjcmVlbi4KPiA+Cj4gPiBVbmZvcnR1bmF0ZWx5LCBjdXJy ZW50bHkgdGhlIG9ubHkgcGxhY2Ugd2UgY2FuIHByb3Blcmx5IGNoZWNrCj4gPiBwb3RlbnRpYWwg bW9kZXMgZm9yIHRoaXMgaXNzdWUgaW4gdGhlIGNvbm5lY3RvciBtb2RlX3ZhbGlkCj4gPiBoZWxw ZXIuIEFnYWluLCBoaWtleSB1c2VzIHRoZSBhZHY3NTExIGJyaWRnZSwgd2hpY2ggaXMgc2hhcmVk Cj4gPiBiZXR3ZWVuIGEgbnVtYmVyIG9mIGRpZmZlcmVudCBkZXZpY2VzLCBzbyBpdHMgaW1wcm9w ZXIgdG8gcHV0Cj4gPiByZXN0cmljdGlvbnMgY2F1c2VkIGJ5IHRoZSBraXJpbiBkcm0gZHJpdmVy IGluIHRoZSBhZHY3NTExCj4gPiBsb2dpYy4KPiA+Cj4gPiBTbyB0aGlzIHBhdGNoIHRyaWVzIHRv IGNvcnJlY3QgZm9yIHRoYXQsIGJ5IGFkZGluZyBzb21lCj4gPiBpbmZyYXN0cnVjdHVyZSBzbyB0 aGF0IHRoZSBkcm1fY3J0Y19oZWxwZXJfZnVuY3MgY2FuIG9wdGlvbmFsbHkKPiA+IGltcGxlbWVu dCBhIG1vZGVfdmFsaWQgY2hlY2ssIHNvIHRoYXQgdGhlIHByb2JlIGhlbHBlcnMgY2FuCj4gPiBj aGVjayB0byBtYWtlIHN1cmUgdGhlcmUgYXJlIG5vdCBhbnkgcmVzdHJpY3Rpb25zIGF0IHRoZSBj cnRjCj4gPiBsZXZlbCBhcyB3ZWxsLgo+ID4KPiA+IENjOiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwu dmV0dGVyQGludGVsLmNvbT4KPiA+IENjOiBKYW5pIE5pa3VsYSA8amFuaS5uaWt1bGFAbGludXgu aW50ZWwuY29tPgo+ID4gQ2M6IFNlYW4gUGF1bCA8c2VhbnBhdWxAY2hyb21pdW0ub3JnPgo+ID4g Q2M6IERhdmlkIEFpcmxpZSA8YWlybGllZEBsaW51eC5pZT4KPiA+IENjOiBSb2IgQ2xhcmsgPHJv YmRjbGFya0BnbWFpbC5jb20+Cj4gPiBDYzogWGlubGlhbmcgTGl1IDx4aW5saWFuZy5saXVAbGlu YXJvLm9yZz4KPiA+IENjOiBYaW5saWFuZyBMaXUgPHoubGl1eGlubGlhbmdAaGlzaWxpY29uLmNv bT4KPiA+IENjOiBSb25ncm9uZyBab3UgPHpvdXJvbmdyb25nQGdtYWlsLmNvbT4KPiA+IENjOiBY aW53ZWkgS29uZyA8a29uZy5rb25neGlud2VpQGhpc2lsaWNvbi5jb20+Cj4gPiBDYzogQ2hlbiBG ZW5nIDxwdWNrLmNoZW5AaGlzaWxpY29uLmNvbT4KPiA+IENjOiBBcmNoaXQgVGFuZWphIDxhcmNo aXR0QGNvZGVhdXJvcmEub3JnPgo+ID4gQ2M6IGRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcKPiA+IFNpZ25lZC1vZmYtYnk6IEpvaG4gU3R1bHR6IDxqb2huLnN0dWx0ekBsaW5hcm8ub3Jn Pgo+IAo+IFNvIEknbSBnb2luZyB0byBiZSBzdXBlci1hbm5veWluZyBoZXJlIGFuZCBhc2sgZm9y IGEgY29tcGxldGUKPiBzb2x1dGlvbi4gVGhpcyBoZXJlIGlzIGRlZmFjdG8gd2hhdCBldmVyIGRy aXZlciBhbHJlYWR5IGRvZXMgKG9yIGhhcwo+IHRvbyksIGJ1dCBpdCBkb2Vzbid0IHJlYWxseSBz b2x2ZSB0aGUgb3ZlcmFsbCBpc3N1ZSBvZiBoYXZpbmcgZW50aXJlbHkKPiBzZXBhcmF0ZSB2YWxp ZGF0aW9uIHBhdGhzIGZvciBwcm9iZSBhbmQgYXRvbWljX2NoZWNrIHBhdGhzLiBJIHRoaW5rIGlm Cj4gd2Ugd2FuIHRvIHNvbHZlIHRoaXMsIHdlIG5lZWQgdG8gc29sdmUgdGhpcyBwcm9wZXJseSwg d2l0aCBhIGdlbmVyaWMKPiBzb2x1dGlvbi4gVGhhdCB3b3VsZCBtZWFuOgo+IC0gc3RpbGwgaW4g aGVscGVycywgdG8gbWFrZSBpdCBhbGwgb3B0LWludAo+IC0gY292ZXJzIGNydGMgYW5kIGVuY29k ZXJzIGFuZCBicmlkZ2VzCj4gLSBhbGxvd3MgeW91IHRvIGltcGxlbWVudCB0aGUgY3VycmVudCBt b2RlX3ZhbGlkIGluIHRlcm1zIG9mIHRoZSBuZXcKPiBzdHVmZiAobWF5YmUgYXMgYSBkZWZhdWx0 IGhvb2spCj4gLSBhbGxvd3MgeW91IHRvIGltcGxlbWVudCB0aGUgY3VycmVudCBhc3NvcnRtZW50 IG9mIG1vZGVfZml4dXAgYW5kL29yCj4gYXRvbWljX2NoZWNrIGluIHRlcm1zIG9mIHRoZSBuZXcg c3R1ZmYsIG9yIGF0IGxlYXN0IHRvIG5vdCBoYXZlIHRvCj4gZHVwbGljYXRlIGxvZ2ljIGluIHRo ZXJlCgpMb25nIGFnbyBJIHF1aWNrbHkgbG9va2VkIGF0IGRvaW5nIHRoaXMgZm9yIGk5MTUuIElJ UkMgdGhlIG1haW4KY29tcGxpY2F0aW9uIHdhcyB0aGUgbm9ybWFsIHZzLiB0aGUgY3J0Y18gdGlt aW5ncyBzdG9yZWQgaW4gdGhlCm1vZGUuIEkgc3VwcG9zZSBvbmUgb3B0aW9uIHdvdWxkIGJlIHRv IHBvcHVsYXRlIHRoZSBjcnRjXyB0aW1pbmdzCmluIC5tb2RlX3ZhbGlkKCkgYXMgd2VsbCBhbmQg b3RoZXJ3aXNlIGp1c3QgaWdub3JlIHRoZSBub3JtYWwgdGltaW5ncy4KCi0tIApWaWxsZSBTeXJq w6Rsw6QKSW50ZWwgT1RDCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9w Lm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1k ZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753812AbdBNTwD (ORCPT ); Tue, 14 Feb 2017 14:52:03 -0500 Received: from mga01.intel.com ([192.55.52.88]:39904 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751389AbdBNTv5 (ORCPT ); Tue, 14 Feb 2017 14:51:57 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,162,1484035200"; d="scan'208";a="1094753746" Date: Tue, 14 Feb 2017 21:51:29 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Daniel Vetter Cc: John Stultz , Chen Feng , lkml , dri-devel , Xinliang Liu , Xinwei Kong , Rongrong Zou , Daniel Vetter Subject: Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs Message-ID: <20170214195129.GN31595@intel.com> References: <1487100302-9445-1-git-send-email-john.stultz@linaro.org> <1487100302-9445-2-git-send-email-john.stultz@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 14, 2017 at 08:38:40PM +0100, Daniel Vetter wrote: > On Tue, Feb 14, 2017 at 8:25 PM, John Stultz wrote: > > Currently, on the hikey board, we have the adv7511 bridge wired > > up to the kirin ade drm driver. Unfortunately, the kirin ade > > core cannot generate accurate byteclocks for all pixel clock > > values. > > > > Thus if a mode clock is selected that we cannot calculate a > > matching byteclock, the device will boot with a blank screen. > > > > Unfortunately, currently the only place we can properly check > > potential modes for this issue in the connector mode_valid > > helper. Again, hikey uses the adv7511 bridge, which is shared > > between a number of different devices, so its improper to put > > restrictions caused by the kirin drm driver in the adv7511 > > logic. > > > > So this patch tries to correct for that, by adding some > > infrastructure so that the drm_crtc_helper_funcs can optionally > > implement a mode_valid check, so that the probe helpers can > > check to make sure there are not any restrictions at the crtc > > level as well. > > > > Cc: Daniel Vetter > > Cc: Jani Nikula > > Cc: Sean Paul > > Cc: David Airlie > > Cc: Rob Clark > > Cc: Xinliang Liu > > Cc: Xinliang Liu > > Cc: Rongrong Zou > > Cc: Xinwei Kong > > Cc: Chen Feng > > Cc: Archit Taneja > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: John Stultz > > So I'm going to be super-annoying here and ask for a complete > solution. This here is defacto what ever driver already does (or has > too), but it doesn't really solve the overall issue of having entirely > separate validation paths for probe and atomic_check paths. I think if > we wan to solve this, we need to solve this properly, with a generic > solution. That would mean: > - still in helpers, to make it all opt-int > - covers crtc and encoders and bridges > - allows you to implement the current mode_valid in terms of the new > stuff (maybe as a default hook) > - allows you to implement the current assortment of mode_fixup and/or > atomic_check in terms of the new stuff, or at least to not have to > duplicate logic in there Long ago I quickly looked at doing this for i915. IIRC the main complication was the normal vs. the crtc_ timings stored in the mode. I suppose one option would be to populate the crtc_ timings in .mode_valid() as well and otherwise just ignore the normal timings. -- Ville Syrjälä Intel OTC