From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751898AbdECGTz (ORCPT ); Wed, 3 May 2017 02:19:55 -0400 Received: from mail-io0-f196.google.com ([209.85.223.196]:35503 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751265AbdECGTi (ORCPT ); Wed, 3 May 2017 02:19:38 -0400 MIME-Version: 1.0 X-Originating-IP: [2a02:168:5640:0:960b:2678:e223:c1c6] In-Reply-To: <80752cfe-4bc7-b7c6-c29b-e634c356a509@synopsys.com> References: <3e1dba0cd5fc053e56f1c9c94d0cb8789ecca4ae.1493203049.git.joabreu@synopsys.com> <20170502084807.k7ioo7a5j33xtnpj@phenom.ffwll.local> <80752cfe-4bc7-b7c6-c29b-e634c356a509@synopsys.com> From: Daniel Vetter Date: Wed, 3 May 2017 08:19:36 +0200 X-Google-Sender-Auth: FvhtQyAl-jjHFOEJrGlo8V7L2AA Message-ID: Subject: Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback To: Jose Abreu Cc: dri-devel , Linux Kernel Mailing List , Carlos Palminha , Alexey Brodkin , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Dave Airlie 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-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v436KELO013289 On Tue, May 2, 2017 at 11:29 AM, Jose Abreu wrote: > On 02-05-2017 09:48, Daniel Vetter wrote: >> On Wed, Apr 26, 2017 at 11:48:34AM +0100, Jose Abreu wrote: >>> Some crtc's may have restrictions in the mode they can display. In >>> this patch a new callback (crtc->mode_valid()) is introduced that >>> is called at the same stage of connector->mode_valid() callback. >>> >>> This shall be implemented if the crtc has some sort of restriction >>> so that we don't probe modes that will fail in the commit() stage. >>> For example: A given crtc may be responsible to set a clock value. >>> If the clock can not produce all the values for the available >>> modes then this callback can be used to restrict the number of >>> probbed modes to only the ones that can be displayed. >>> >>> If the crtc does not implement the callback then the behaviour will >>> remain the same. Also, for a given set of crtcs that can be bound to >>> the connector, if at least one can display the mode then the mode >>> will be probbed. >>> >>> Signed-off-by: Jose Abreu >>> Cc: Carlos Palminha >>> Cc: Alexey Brodkin >>> Cc: Ville Syrjälä >>> Cc: Daniel Vetter >>> Cc: Dave Airlie >> Not sure this is useful, since you still have to duplicate the exact same >> check into your ->mode_fixup hook. That seems to make things even more >> confusing. > > Yeah, in arcpgu I had to duplicate the code in ->atomic_check. > >> >> Also this doesn't update the various kerneldoc comments. For the existing >> hooks. Since this topic causes so much confusion, I don't think a >> half-solution will help, but has some good potential to make things worse. > > I only documented the callback in drm_modeset_helper_vtables.h. > > Despite all of this, I think it doesn't makes sense delivering > modes to userspace which can never be used. > > This is really annoying in arcpgu. Imagine: I try to use mpv to > play a video, the full set of modes from EDID were probed so if I > just start mpv it will pick the native mode of the TV instead of > the one that is supported, so mpv will fail to play. I know the > value of clock which will work (so I know what mode shall be > used), but a normal user which is not aware of the HW will have > to cycle through the list of modes and try them all until it hits > one that works. Its really boring. > > For the modes that user specifies manually there is nothing we > can do, but we should not trick users into thinking that a given > mode is supported when it will always fail at commit. Yes, you are supposed to filter these out in ->mode_valid. But my stance is that only adding a half-baked support for a new callback to the core isn't going to make life easier for drivers, it will just add to the confusion. There's already piles of docs for both @mode_valid and @mode_fixup hooks explaining this, I don't want to make the documentation even more complex. And half-baked crtc checking is _much_ easier to implement in the driver directly (e.g. i915 checks for crtc constraints since forever, as do the other big x86 drivers). So all taken together, if we add a ->mode_valid to crtcs, then imo we should do it right and actually make life easier for drivers. A good proof would be if your patch would allow us to drop a lot of the lenghty language from the @mode_valid hooks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback Date: Wed, 3 May 2017 08:19:36 +0200 Message-ID: References: <3e1dba0cd5fc053e56f1c9c94d0cb8789ecca4ae.1493203049.git.joabreu@synopsys.com> <20170502084807.k7ioo7a5j33xtnpj@phenom.ffwll.local> <80752cfe-4bc7-b7c6-c29b-e634c356a509@synopsys.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-io0-x243.google.com (mail-io0-x243.google.com [IPv6:2607:f8b0:4001:c06::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 25CB26E296 for ; Wed, 3 May 2017 06:19:38 +0000 (UTC) Received: by mail-io0-x243.google.com with SMTP id x86so34201110ioe.3 for ; Tue, 02 May 2017 23:19:38 -0700 (PDT) In-Reply-To: <80752cfe-4bc7-b7c6-c29b-e634c356a509@synopsys.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jose Abreu Cc: Alexey Brodkin , Linux Kernel Mailing List , dri-devel , Carlos Palminha List-Id: dri-devel@lists.freedesktop.org T24gVHVlLCBNYXkgMiwgMjAxNyBhdCAxMToyOSBBTSwgSm9zZSBBYnJldSA8Sm9zZS5BYnJldUBz eW5vcHN5cy5jb20+IHdyb3RlOgo+IE9uIDAyLTA1LTIwMTcgMDk6NDgsIERhbmllbCBWZXR0ZXIg d3JvdGU6Cj4+IE9uIFdlZCwgQXByIDI2LCAyMDE3IGF0IDExOjQ4OjM0QU0gKzAxMDAsIEpvc2Ug QWJyZXUgd3JvdGU6Cj4+PiBTb21lIGNydGMncyBtYXkgaGF2ZSByZXN0cmljdGlvbnMgaW4gdGhl IG1vZGUgdGhleSBjYW4gZGlzcGxheS4gSW4KPj4+IHRoaXMgcGF0Y2ggYSBuZXcgY2FsbGJhY2sg KGNydGMtPm1vZGVfdmFsaWQoKSkgaXMgaW50cm9kdWNlZCB0aGF0Cj4+PiBpcyBjYWxsZWQgYXQg dGhlIHNhbWUgc3RhZ2Ugb2YgY29ubmVjdG9yLT5tb2RlX3ZhbGlkKCkgY2FsbGJhY2suCj4+Pgo+ Pj4gVGhpcyBzaGFsbCBiZSBpbXBsZW1lbnRlZCBpZiB0aGUgY3J0YyBoYXMgc29tZSBzb3J0IG9m IHJlc3RyaWN0aW9uCj4+PiBzbyB0aGF0IHdlIGRvbid0IHByb2JlIG1vZGVzIHRoYXQgd2lsbCBm YWlsIGluIHRoZSBjb21taXQoKSBzdGFnZS4KPj4+IEZvciBleGFtcGxlOiBBIGdpdmVuIGNydGMg bWF5IGJlIHJlc3BvbnNpYmxlIHRvIHNldCBhIGNsb2NrIHZhbHVlLgo+Pj4gSWYgdGhlIGNsb2Nr IGNhbiBub3QgcHJvZHVjZSBhbGwgdGhlIHZhbHVlcyBmb3IgdGhlIGF2YWlsYWJsZQo+Pj4gbW9k ZXMgdGhlbiB0aGlzIGNhbGxiYWNrIGNhbiBiZSB1c2VkIHRvIHJlc3RyaWN0IHRoZSBudW1iZXIg b2YKPj4+IHByb2JiZWQgbW9kZXMgdG8gb25seSB0aGUgb25lcyB0aGF0IGNhbiBiZSBkaXNwbGF5 ZWQuCj4+Pgo+Pj4gSWYgdGhlIGNydGMgZG9lcyBub3QgaW1wbGVtZW50IHRoZSBjYWxsYmFjayB0 aGVuIHRoZSBiZWhhdmlvdXIgd2lsbAo+Pj4gcmVtYWluIHRoZSBzYW1lLiBBbHNvLCBmb3IgYSBn aXZlbiBzZXQgb2YgY3J0Y3MgdGhhdCBjYW4gYmUgYm91bmQgdG8KPj4+IHRoZSBjb25uZWN0b3Is IGlmIGF0IGxlYXN0IG9uZSBjYW4gZGlzcGxheSB0aGUgbW9kZSB0aGVuIHRoZSBtb2RlCj4+PiB3 aWxsIGJlIHByb2JiZWQuCj4+Pgo+Pj4gU2lnbmVkLW9mZi1ieTogSm9zZSBBYnJldSA8am9hYnJl dUBzeW5vcHN5cy5jb20+Cj4+PiBDYzogQ2FybG9zIFBhbG1pbmhhIDxwYWxtaW5oYUBzeW5vcHN5 cy5jb20+Cj4+PiBDYzogQWxleGV5IEJyb2RraW4gPGFicm9ka2luQHN5bm9wc3lzLmNvbT4KPj4+ IENjOiBWaWxsZSBTeXJqw6Rsw6QgPHZpbGxlLnN5cmphbGFAbGludXguaW50ZWwuY29tPgo+Pj4g Q2M6IERhbmllbCBWZXR0ZXIgPGRhbmllbC52ZXR0ZXJAZmZ3bGwuY2g+Cj4+PiBDYzogRGF2ZSBB aXJsaWUgPGFpcmxpZWRAbGludXguaWU+Cj4+IE5vdCBzdXJlIHRoaXMgaXMgdXNlZnVsLCBzaW5j ZSB5b3Ugc3RpbGwgaGF2ZSB0byBkdXBsaWNhdGUgdGhlIGV4YWN0IHNhbWUKPj4gY2hlY2sgaW50 byB5b3VyIC0+bW9kZV9maXh1cCBob29rLiBUaGF0IHNlZW1zIHRvIG1ha2UgdGhpbmdzIGV2ZW4g bW9yZQo+PiBjb25mdXNpbmcuCj4KPiBZZWFoLCBpbiBhcmNwZ3UgSSBoYWQgdG8gZHVwbGljYXRl IHRoZSBjb2RlIGluIC0+YXRvbWljX2NoZWNrLgo+Cj4+Cj4+IEFsc28gdGhpcyBkb2Vzbid0IHVw ZGF0ZSB0aGUgdmFyaW91cyBrZXJuZWxkb2MgY29tbWVudHMuIEZvciB0aGUgZXhpc3RpbmcKPj4g aG9va3MuIFNpbmNlIHRoaXMgdG9waWMgY2F1c2VzIHNvIG11Y2ggY29uZnVzaW9uLCBJIGRvbid0 IHRoaW5rIGEKPj4gaGFsZi1zb2x1dGlvbiB3aWxsIGhlbHAsIGJ1dCBoYXMgc29tZSBnb29kIHBv dGVudGlhbCB0byBtYWtlIHRoaW5ncyB3b3JzZS4KPgo+IEkgb25seSBkb2N1bWVudGVkIHRoZSBj YWxsYmFjayBpbiBkcm1fbW9kZXNldF9oZWxwZXJfdnRhYmxlcy5oLgo+Cj4gRGVzcGl0ZSBhbGwg b2YgdGhpcywgSSB0aGluayBpdCBkb2Vzbid0IG1ha2VzIHNlbnNlIGRlbGl2ZXJpbmcKPiBtb2Rl cyB0byB1c2Vyc3BhY2Ugd2hpY2ggY2FuIG5ldmVyIGJlIHVzZWQuCj4KPiBUaGlzIGlzIHJlYWxs eSBhbm5veWluZyBpbiBhcmNwZ3UuIEltYWdpbmU6IEkgdHJ5IHRvIHVzZSBtcHYgdG8KPiBwbGF5 IGEgdmlkZW8sIHRoZSBmdWxsIHNldCBvZiBtb2RlcyBmcm9tIEVESUQgd2VyZSBwcm9iZWQgc28g aWYgSQo+IGp1c3Qgc3RhcnQgbXB2IGl0IHdpbGwgcGljayB0aGUgbmF0aXZlIG1vZGUgb2YgdGhl IFRWIGluc3RlYWQgb2YKPiB0aGUgb25lIHRoYXQgaXMgc3VwcG9ydGVkLCBzbyBtcHYgd2lsbCBm YWlsIHRvIHBsYXkuIEkga25vdyB0aGUKPiB2YWx1ZSBvZiBjbG9jayB3aGljaCB3aWxsIHdvcmsg KHNvIEkga25vdyB3aGF0IG1vZGUgc2hhbGwgYmUKPiB1c2VkKSwgYnV0IGEgbm9ybWFsIHVzZXIg d2hpY2ggaXMgbm90IGF3YXJlIG9mIHRoZSBIVyB3aWxsIGhhdmUKPiB0byBjeWNsZSB0aHJvdWdo IHRoZSBsaXN0IG9mIG1vZGVzIGFuZCB0cnkgdGhlbSBhbGwgdW50aWwgaXQgaGl0cwo+IG9uZSB0 aGF0IHdvcmtzLiBJdHMgcmVhbGx5IGJvcmluZy4KPgo+IEZvciB0aGUgbW9kZXMgdGhhdCB1c2Vy IHNwZWNpZmllcyBtYW51YWxseSB0aGVyZSBpcyBub3RoaW5nIHdlCj4gY2FuIGRvLCBidXQgd2Ug c2hvdWxkIG5vdCB0cmljayB1c2VycyBpbnRvIHRoaW5raW5nIHRoYXQgYSBnaXZlbgo+IG1vZGUg aXMgc3VwcG9ydGVkIHdoZW4gaXQgd2lsbCBhbHdheXMgZmFpbCBhdCBjb21taXQuCgpZZXMsIHlv dSBhcmUgc3VwcG9zZWQgdG8gZmlsdGVyIHRoZXNlIG91dCBpbiAtPm1vZGVfdmFsaWQuIEJ1dCBt eQpzdGFuY2UgaXMgdGhhdCBvbmx5IGFkZGluZyBhIGhhbGYtYmFrZWQgc3VwcG9ydCBmb3IgYSBu ZXcgY2FsbGJhY2sgdG8KdGhlIGNvcmUgaXNuJ3QgZ29pbmcgdG8gbWFrZSBsaWZlIGVhc2llciBm b3IgZHJpdmVycywgaXQgd2lsbCBqdXN0IGFkZAp0byB0aGUgY29uZnVzaW9uLiBUaGVyZSdzIGFs cmVhZHkgcGlsZXMgb2YgZG9jcyBmb3IgYm90aCBAbW9kZV92YWxpZAphbmQgQG1vZGVfZml4dXAg aG9va3MgZXhwbGFpbmluZyB0aGlzLCBJIGRvbid0IHdhbnQgdG8gbWFrZSB0aGUKZG9jdW1lbnRh dGlvbiBldmVuIG1vcmUgY29tcGxleC4gQW5kIGhhbGYtYmFrZWQgY3J0YyBjaGVja2luZyBpcwpf bXVjaF8gZWFzaWVyIHRvIGltcGxlbWVudCBpbiB0aGUgZHJpdmVyIGRpcmVjdGx5IChlLmcuIGk5 MTUgY2hlY2tzCmZvciBjcnRjIGNvbnN0cmFpbnRzIHNpbmNlIGZvcmV2ZXIsIGFzIGRvIHRoZSBv dGhlciBiaWcgeDg2IGRyaXZlcnMpLgoKU28gYWxsIHRha2VuIHRvZ2V0aGVyLCBpZiB3ZSBhZGQg YSAtPm1vZGVfdmFsaWQgdG8gY3J0Y3MsIHRoZW4gaW1vIHdlCnNob3VsZCBkbyBpdCByaWdodCBh bmQgYWN0dWFsbHkgbWFrZSBsaWZlIGVhc2llciBmb3IgZHJpdmVycy4gQSBnb29kCnByb29mIHdv dWxkIGJlIGlmIHlvdXIgcGF0Y2ggd291bGQgYWxsb3cgdXMgdG8gZHJvcCBhIGxvdCBvZiB0aGUK bGVuZ2h0eSBsYW5ndWFnZSBmcm9tIHRoZSBAbW9kZV92YWxpZCBob29rcy4KLURhbmllbAotLSAK RGFuaWVsIFZldHRlcgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24KKzQxICgw KSA3OSAzNjUgNTcgNDggLSBodHRwOi8vYmxvZy5mZndsbC5jaApfX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1k ZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcv bWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK