From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set Date: Fri, 17 Mar 2017 13:13:59 +0200 Message-ID: <20170317111359.GR31595@intel.com> References: <20170316095553.1586-1-michel@daenzer.net> <20170316100900.GC31595@intel.com> <87d59360-8419-b547-7b6b-32afedcf1330@daenzer.net> <20170317100133.GO31595@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id C41056E1D7 for ; Fri, 17 Mar 2017 11:14:02 +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: Michel =?iso-8859-1?Q?D=E4nzer?= Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBNYXIgMTcsIDIwMTcgYXQgMDc6MTk6MjdQTSArMDkwMCwgTWljaGVsIETDpG56ZXIg d3JvdGU6Cj4gT24gMTcvMDMvMTcgMDc6MDEgUE0sIFZpbGxlIFN5cmrDpGzDpCB3cm90ZToKPiA+ IE9uIEZyaSwgTWFyIDE3LCAyMDE3IGF0IDA2OjAxOjQxUE0gKzA5MDAsIE1pY2hlbCBEw6RuemVy IHdyb3RlOgo+ID4+IE9uIDE2LzAzLzE3IDA3OjA5IFBNLCBWaWxsZSBTeXJqw6Rsw6Qgd3JvdGU6 Cj4gPj4+IE9uIFRodSwgTWFyIDE2LCAyMDE3IGF0IDA2OjU1OjUzUE0gKzA5MDAsIE1pY2hlbCBE w6RuemVyIHdyb3RlOgo+ID4+Pj4gRnJvbTogTWljaGVsIETDpG56ZXIgPG1pY2hlbC5kYWVuemVy QGFtZC5jb20+Cj4gPj4+Pgo+ID4+Pj4gT3RoZXJ3aXNlIHRoaXMgY2FuIGFsc28gcHJldmVudCBt b2Rlc2V0cyBlLmcuIGZvciBzd2l0Y2hpbmcgVlRzLgo+ID4+Pj4KPiA+Pj4+IEZCX01JU0NfVVNF Ul9FVkVOVCBpcyBzZXQgd2hlbiB0aGUgcmVxdWVzdCBvcmlnaW5hdGVzIGZyb20gdXNlcnNwYWNl LAo+ID4+Pj4gd2hpY2ggaXMgd2hhdCB3ZSdyZSBpbnRlcmVzdGVkIGluIGhlcmUgYWNjb3JkaW5n IHRvIHRoZSBEUk1fREVCVUcKPiA+Pj4+IG91dHB1dC4KPiA+Pj4KPiA+Pj4gU28gd2h5IGlzIHRo ZSBrZXJuZWwgYWxsb3dlZCB0byB2aW9sYXRlIHRoaXM/Cj4gPj4+Cj4gPj4+IFRoZSBjaGVja3Mg bG9vayBzb21ld2hhdCBib2d1cyB0byBtZSBhbnl3YXkuIFRoZSAndmlydHVhbCBzaXplID09IGZi IHNpemUnCj4gPj4+IGNoZWNrIG1ha2VzIHNvbWUga2luZCBvZiBzZW5zZS4gQWx0aG91Z2ggSSBk b24ndCBzZWUgd2h5IHRoZSB2aXJ0dWFsCj4gPj4+IHJlc29sdXRpb24gY291bGRuJ3QgYmUgc21h bGxlciB0aGFuIHRoZSBmYiBzaXplLiBCdXQgcmVxdWlyaW5nIHRoYXQgdGhlCj4gPj4+IHZpc2li bGUgcmVzb2x1dGlvbm4gbWF0Y2hlcyB0aGUgZmIgc2l6ZSBhcyB3ZWxsIGRlZmluaXRlbHkgc2Vl bXMgd3JvbmcuCj4gPj4KPiA+PiBBZ3JlZWQgb24gYWxsIGNvdW50cy4gU28sIEkgdGhpbmsgd2hh dCdzIG5lZWRlZCBpcyBhbG1vc3QgYSByZXZlcnQgb2YKPiA+PiA4NjVhZmIxMTk0OWUsIGV4Y2Vw dCBmb3IgdGhlIGJpdHNfcGVyX3BpeGVsIGNvbXBhcmlzb24sIHNpbmNlIHdlIHJlYWxseQo+ID4+ IGNhbid0IGNoYW5nZSB0aGF0LiBTZWUgZGlmZiBiZWxvdy4KPiA+IAo+ID4gU28gb25lIHNlbWkt Y3JhenkgaWRlYSBJIGhhZCB3YXM6Cj4gPiAxMjoxOCA8IHZzeXJqYWxhPiBkYW5pZWxzOiBobW0u IGdpdmVuIHRoYXQgdGhlIGZiX2hlbHBlciBkb2Vzbid0Cj4gPiAgaW1wbGVtZW50IGEgbW9kZXNl dCBpbiBzZXRfcGFyLCBpIGd1ZXNzIHdoYXQgd2UgcmVhbGx5Cj4gPiAgc2hvdWxkIGRvIGlzIGxv b2sgYXQgdGhlIGN1cnJlbnRseSBhY3RpdmUgY3J0Y3MgYW5kIHNlZSBpZiB0aGUgbW9kZSBvbgo+ ID4gIG9uZSBvZiB0aGVtIG1vcmUgb3IgbGVzcyBtYXRjaGVzIHdoYXQgdGhlIHVzZXIgaXMgYXNr aW5nIGZvcgo+IAo+IEkgZG9uJ3QgZ2V0IHRoZSBpZGVhLiBXaGF0J3MgdGhlIHBvaW50IG9mIGNv bXBhcmluZyB0aGUgdmFyLT4qIHZhbHVlcyB0bwo+IHdoYXRldmVyIGlzIGN1cnJlbnRseSBhY3Rp dmUgaW4gdGhlIGhhcmR3YXJlPwoKTm90IGN1cnJlbnRseSBhY3RpdmUgaW4gdGhlIGhhcmR3YXJl LCBidXQgY3VycmVudGx5IGFjdGl2ZSBhcyBmYXIgYXMKZmJfaGVscGVyIGlzIGNvbmNlcm5lZC4g SSBndWVzcyBJIHNob3VsZCBzYXkgIm1hdGNoZXMgZmJfaGVscGVyJ3MKY3VycmVudCBjcnRjIGNv bmZpZ3VyYXRpb24iIG9yIHNvbWV0aGluZy4KCj4gVGhlIHB1cnBvc2Ugb2YgdGhlIGNvZGUgaXMK PiB0byBjaGVjayB0aGF0IHRoZSByZXF1ZXN0ZWQgcGFyYW1ldGVycyBmaXQgaW50byBmYl9oZWxw ZXIncyBmcmFtZWJ1ZmZlci4KPiAKPiAKPiA+IEkgdHJpZWQgdG8gdHJpcCB0aGlzIGN1cnJlbnQg Y2hlY2sgYnkgYm9vdGluZyB3aXRoIGEgYmlnIHNjcmVlbiBhbmQKPiA+IGxhdGVyIHN3aXRjaGlu ZyB0byBhIHNtYWxsIHNjcmVlbi4gRm9yIHNvbWUgcmVhc29uIHRoYXQgd29ya2VkIG91dAo+ID4g anVzdCBmaW5lLAo+IAo+IEknZCBuZWVkIG1vcmUgZGV0YWlscyBhYm91dCB3aGF0IGV4YWN0bHkg eW91IHRyaWVkLgo+IAo+ID4gc28gSSdtIG5vdCBldmVuIHN1cmUgd2hhdCBraW5kIG9mIHZhbHVl cyB3ZSBzdHVmZiBpbnRvIHhyZXMgJiBjby4KPiAKPiBJdCBzaG91bGQgYmUgdGhlIHZhbHVlcyBz aG93biAvIGF0dGVtcHRlZCB0byBzZXQgYnkgZmJzZXQuCgpJIG1lYW50IHdoYXQgZG9lcyB0aGUg a2VybmVsIHB1dCB0aGVyZSBmb3IgZmJjb24gZXRjLiBJIHdhcyBleHBlY3RpbmcKdGhhdCBpdCB4 cmVzL3lyZXMgd291bGQgYmUgdGhlIHZpc2libGUgcmVzb2x1dGlvbiBvZiB0aGUgc21hbGxlc3QK c2NyZWVuLCBhbmQgdGhlIHZpcnR1YWwgcmVzb2x1dGlvbiB3b3VsZCBiZSBlaXRoZXIgdGhlIHNh bWUgb3IgdGhlCnNpemUgb2YgdGhlIGZiIHBlcmhhcHMuIEJ1dCB0aGF0IGNhbid0IGJlIHRoZSBj YXNlIG9yIGVsc2UgbXkgZXhwZXJpbWVudAp3b3VsZCBoYXZlIHByb2R1Y2VkIGEgZmFpbHVyZS4g U28gSSB3b3VsZCBoYXZlIHRvIGR1bXAgdGhhdCBzdHVmZiBvdXQgdG8Kc2VlIHdoYXQgcmVhbGx5 IGVuZHMgdXAgdGhlcmUuCgotLSAKVmlsbGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBs aXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVz a3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751455AbdCQLOM (ORCPT ); Fri, 17 Mar 2017 07:14:12 -0400 Received: from mga09.intel.com ([134.134.136.24]:29568 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750952AbdCQLOJ (ORCPT ); Fri, 17 Mar 2017 07:14:09 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,176,1486454400"; d="scan'208";a="1109513487" Date: Fri, 17 Mar 2017 13:13:59 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Michel =?iso-8859-1?Q?D=E4nzer?= Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set Message-ID: <20170317111359.GR31595@intel.com> References: <20170316095553.1586-1-michel@daenzer.net> <20170316100900.GC31595@intel.com> <87d59360-8419-b547-7b6b-32afedcf1330@daenzer.net> <20170317100133.GO31595@intel.com> 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 Fri, Mar 17, 2017 at 07:19:27PM +0900, Michel Dänzer wrote: > On 17/03/17 07:01 PM, Ville Syrjälä wrote: > > On Fri, Mar 17, 2017 at 06:01:41PM +0900, Michel Dänzer wrote: > >> On 16/03/17 07:09 PM, Ville Syrjälä wrote: > >>> On Thu, Mar 16, 2017 at 06:55:53PM +0900, Michel Dänzer wrote: > >>>> From: Michel Dänzer > >>>> > >>>> Otherwise this can also prevent modesets e.g. for switching VTs. > >>>> > >>>> FB_MISC_USER_EVENT is set when the request originates from userspace, > >>>> which is what we're interested in here according to the DRM_DEBUG > >>>> output. > >>> > >>> So why is the kernel allowed to violate this? > >>> > >>> The checks look somewhat bogus to me anyway. The 'virtual size == fb size' > >>> check makes some kind of sense. Although I don't see why the virtual > >>> resolution couldn't be smaller than the fb size. But requiring that the > >>> visible resolutionn matches the fb size as well definitely seems wrong. > >> > >> Agreed on all counts. So, I think what's needed is almost a revert of > >> 865afb11949e, except for the bits_per_pixel comparison, since we really > >> can't change that. See diff below. > > > > So one semi-crazy idea I had was: > > 12:18 < vsyrjala> daniels: hmm. given that the fb_helper doesn't > > implement a modeset in set_par, i guess what we really > > should do is look at the currently active crtcs and see if the mode on > > one of them more or less matches what the user is asking for > > I don't get the idea. What's the point of comparing the var->* values to > whatever is currently active in the hardware? Not currently active in the hardware, but currently active as far as fb_helper is concerned. I guess I should say "matches fb_helper's current crtc configuration" or something. > The purpose of the code is > to check that the requested parameters fit into fb_helper's framebuffer. > > > > I tried to trip this current check by booting with a big screen and > > later switching to a small screen. For some reason that worked out > > just fine, > > I'd need more details about what exactly you tried. > > > so I'm not even sure what kind of values we stuff into xres & co. > > It should be the values shown / attempted to set by fbset. I meant what does the kernel put there for fbcon etc. I was expecting that it xres/yres would be the visible resolution of the smallest screen, and the virtual resolution would be either the same or the size of the fb perhaps. But that can't be the case or else my experiment would have produced a failure. So I would have to dump that stuff out to see what really ends up there. -- Ville Syrjälä Intel OTC