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 12:01:33 +0200 Message-ID: <20170317100133.GO31595@intel.com> References: <20170316095553.1586-1-michel@daenzer.net> <20170316100900.GC31595@intel.com> <87d59360-8419-b547-7b6b-32afedcf1330@daenzer.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id A92026ECC8 for ; Fri, 17 Mar 2017 10:01:37 +0000 (UTC) Content-Disposition: inline In-Reply-To: <87d59360-8419-b547-7b6b-32afedcf1330@daenzer.net> 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 T24gRnJpLCBNYXIgMTcsIDIwMTcgYXQgMDY6MDE6NDFQTSArMDkwMCwgTWljaGVsIETDpG56ZXIg d3JvdGU6Cj4gT24gMTYvMDMvMTcgMDc6MDkgUE0sIFZpbGxlIFN5cmrDpGzDpCB3cm90ZToKPiA+ IE9uIFRodSwgTWFyIDE2LCAyMDE3IGF0IDA2OjU1OjUzUE0gKzA5MDAsIE1pY2hlbCBEw6RuemVy IHdyb3RlOgo+ID4+IEZyb206IE1pY2hlbCBEw6RuemVyIDxtaWNoZWwuZGFlbnplckBhbWQuY29t Pgo+ID4+Cj4gPj4gT3RoZXJ3aXNlIHRoaXMgY2FuIGFsc28gcHJldmVudCBtb2Rlc2V0cyBlLmcu IGZvciBzd2l0Y2hpbmcgVlRzLgo+ID4+Cj4gPj4gRkJfTUlTQ19VU0VSX0VWRU5UIGlzIHNldCB3 aGVuIHRoZSByZXF1ZXN0IG9yaWdpbmF0ZXMgZnJvbSB1c2Vyc3BhY2UsCj4gPj4gd2hpY2ggaXMg d2hhdCB3ZSdyZSBpbnRlcmVzdGVkIGluIGhlcmUgYWNjb3JkaW5nIHRvIHRoZSBEUk1fREVCVUcK PiA+PiBvdXRwdXQuCj4gPiAKPiA+IFNvIHdoeSBpcyB0aGUga2VybmVsIGFsbG93ZWQgdG8gdmlv bGF0ZSB0aGlzPwo+ID4gCj4gPiBUaGUgY2hlY2tzIGxvb2sgc29tZXdoYXQgYm9ndXMgdG8gbWUg YW55d2F5LiBUaGUgJ3ZpcnR1YWwgc2l6ZSA9PSBmYiBzaXplJwo+ID4gY2hlY2sgbWFrZXMgc29t ZSBraW5kIG9mIHNlbnNlLiBBbHRob3VnaCBJIGRvbid0IHNlZSB3aHkgdGhlIHZpcnR1YWwKPiA+ IHJlc29sdXRpb24gY291bGRuJ3QgYmUgc21hbGxlciB0aGFuIHRoZSBmYiBzaXplLiBCdXQgcmVx dWlyaW5nIHRoYXQgdGhlCj4gPiB2aXNpYmxlIHJlc29sdXRpb25uIG1hdGNoZXMgdGhlIGZiIHNp emUgYXMgd2VsbCBkZWZpbml0ZWx5IHNlZW1zIHdyb25nLgo+IAo+IEFncmVlZCBvbiBhbGwgY291 bnRzLiBTbywgSSB0aGluayB3aGF0J3MgbmVlZGVkIGlzIGFsbW9zdCBhIHJldmVydCBvZgo+IDg2 NWFmYjExOTQ5ZSwgZXhjZXB0IGZvciB0aGUgYml0c19wZXJfcGl4ZWwgY29tcGFyaXNvbiwgc2lu Y2Ugd2UgcmVhbGx5Cj4gY2FuJ3QgY2hhbmdlIHRoYXQuIFNlZSBkaWZmIGJlbG93LgoKU28gb25l IHNlbWktY3JhenkgaWRlYSBJIGhhZCB3YXM6CjEyOjE4IDwgdnN5cmphbGE+IGRhbmllbHM6IGht bS4gZ2l2ZW4gdGhhdCB0aGUgZmJfaGVscGVyIGRvZXNuJ3QKIGltcGxlbWVudCBhIG1vZGVzZXQg aW4gc2V0X3BhciwgaSBndWVzcyB3aGF0IHdlIHJlYWxseQogc2hvdWxkIGRvIGlzIGxvb2sgYXQg dGhlIGN1cnJlbnRseSBhY3RpdmUgY3J0Y3MgYW5kIHNlZSBpZiB0aGUgbW9kZSBvbgogb25lIG9m IHRoZW0gbW9yZSBvciBsZXNzIG1hdGNoZXMgd2hhdCB0aGUgdXNlciBpcyBhc2tpbmcgZm9yCgpJ IHRyaWVkIHRvIHRyaXAgdGhpcyBjdXJyZW50IGNoZWNrIGJ5IGJvb3Rpbmcgd2l0aCBhIGJpZyBz Y3JlZW4gYW5kCmxhdGVyIHN3aXRjaGluZyB0byBhIHNtYWxsIHNjcmVlbi4gRm9yIHNvbWUgcmVh c29uIHRoYXQgd29ya2VkIG91dApqdXN0IGZpbmUsIHNvIEknbSBub3QgZXZlbiBzdXJlIHdoYXQg a2luZCBvZiB2YWx1ZXMgd2Ugc3R1ZmYgaW50bwp4cmVzICYgY28uCgo+IAo+IERvZXMgdGhhdCBt YWtlIHNlbnNlPyBTdGVmYW4sIHdvdWxkIHRoaXMgYnJlYWsgYW55IHRlc3QgY2FzZSB5b3Ugd3Jv dGUKPiB5b3VyIHBhdGNoIGZvcj8KPiAKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJt L2RybV9mYl9oZWxwZXIuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fZmJfaGVscGVyLmMKPiBpbmRl eCBmNmQ0ZDk3MDA3MzQuLmY0ZjcwY2UyNGZjYyAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9k cm0vZHJtX2ZiX2hlbHBlci5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2RybV9mYl9oZWxwZXIu Ywo+IEBAIC0xMjYwLDggKzEyNjAsOCBAQCBpbnQgZHJtX2ZiX2hlbHBlcl9jaGVja192YXIoc3Ry dWN0IGZiX3Zhcl9zY3JlZW5pbmZvICp2YXIsCj4gICAgICAgICAgKiB0byBLTVMsIGhlbmNlIGZh aWwgaWYgZGlmZmVyZW50IHNldHRpbmdzIGFyZSByZXF1ZXN0ZWQuCj4gICAgICAgICAgKi8KPiAg ICAgICAgIGlmICh2YXItPmJpdHNfcGVyX3BpeGVsICE9IGZiLT5mb3JtYXQtPmNwcFswXSAqIDgg fHwKPiAtICAgICAgICAgICB2YXItPnhyZXMgIT0gZmItPndpZHRoIHx8IHZhci0+eXJlcyAhPSBm Yi0+aGVpZ2h0IHx8Cj4gLSAgICAgICAgICAgdmFyLT54cmVzX3ZpcnR1YWwgIT0gZmItPndpZHRo IHx8IHZhci0+eXJlc192aXJ0dWFsICE9IGZiLT5oZWlnaHQpIHsKPiArICAgICAgICAgICB2YXIt PnhyZXMgPiBmYi0+d2lkdGggfHwgdmFyLT55cmVzID4gZmItPmhlaWdodCB8fAo+ICsgICAgICAg ICAgIHZhci0+eHJlc192aXJ0dWFsID4gZmItPndpZHRoIHx8IHZhci0+eXJlc192aXJ0dWFsID4g ZmItPmhlaWdodCkgewo+ICAgICAgICAgICAgICAgICBEUk1fREVCVUcoImZiIHVzZXJzcGFjZSBy ZXF1ZXN0ZWQgd2lkdGgvaGVpZ2h0L2JwcCBkaWZmZXJlbnQgdGhhbiBjdXJyZW50IGZiICIKPiAg ICAgICAgICAgICAgICAgICAgICAgICAgICJyZXF1ZXN0ICVkeCVkLSVkICh2aXJ0dWFsICVkeCVk KSA+ICVkeCVkLSVkXG4iLAo+ICAgICAgICAgICAgICAgICAgICAgICAgICAgdmFyLT54cmVzLCB2 YXItPnlyZXMsIHZhci0+Yml0c19wZXJfcGl4ZWwsCj4gCj4gCj4gCj4gPj4gQnVnemlsbGE6IGh0 dHBzOi8vYnVncy5mcmVlZGVza3RvcC5vcmcvOTk4NDEKPiA+PiBGaXhlczogODY1YWZiMTE5NDll ICgiZHJtL2ZiLWhlbHBlcjogcmVqZWN0IGFueSBjaGFuZ2VzIHRvIHRoZSBmYmRldiIpCj4gPj4g U2lnbmVkLW9mZi1ieTogTWljaGVsIETDpG56ZXIgPG1pY2hlbC5kYWVuemVyQGFtZC5jb20+Cj4g Pj4gLS0tCj4gPj4KPiA+PiBJJ20gbm90IGVudGlyZWx5IHN1cmUgd2h5IHRoZSB2YWx1ZXMgY2Fu IG5vdCBtYXRjaCBmb3IgYSBWVCBzd2l0Y2guIElmCj4gPj4gYW55Ym9keSB0aGlua3MgdGhpcyBq dXN0IHBhcGVycyBvdmVyIHRoZSByZWFsIGlzc3VlLCBwbGVhc2Ugc3BlYWsgdXAuCj4gPj4KPiA+ PiAgZHJpdmVycy9ncHUvZHJtL2RybV9mYl9oZWxwZXIuYyB8IDcgKysrKy0tLQo+ID4+ICAxIGZp bGUgY2hhbmdlZCwgNCBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQo+ID4+Cj4gPj4gZGlm ZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fZmJfaGVscGVyLmMgYi9kcml2ZXJzL2dwdS9k cm0vZHJtX2ZiX2hlbHBlci5jCj4gPj4gaW5kZXggZjZkNGQ5NzAwNzM0Li45NjYzZjNiOGYyODcg MTAwNjQ0Cj4gPj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2RybV9mYl9oZWxwZXIuYwo+ID4+ICsr KyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fZmJfaGVscGVyLmMKPiA+PiBAQCAtMTI1OSw5ICsxMjU5 LDEwIEBAIGludCBkcm1fZmJfaGVscGVyX2NoZWNrX3ZhcihzdHJ1Y3QgZmJfdmFyX3NjcmVlbmlu Zm8gKnZhciwKPiA+PiAgCSAqIENoYW5nZXMgc3RydWN0IGZiX3Zhcl9zY3JlZW5pbmZvIGFyZSBj dXJyZW50bHkgbm90IHB1c2hlZCBiYWNrCj4gPj4gIAkgKiB0byBLTVMsIGhlbmNlIGZhaWwgaWYg ZGlmZmVyZW50IHNldHRpbmdzIGFyZSByZXF1ZXN0ZWQuCj4gPj4gIAkgKi8KPiA+PiAtCWlmICh2 YXItPmJpdHNfcGVyX3BpeGVsICE9IGZiLT5mb3JtYXQtPmNwcFswXSAqIDggfHwKPiA+PiAtCSAg ICB2YXItPnhyZXMgIT0gZmItPndpZHRoIHx8IHZhci0+eXJlcyAhPSBmYi0+aGVpZ2h0IHx8Cj4g Pj4gLQkgICAgdmFyLT54cmVzX3ZpcnR1YWwgIT0gZmItPndpZHRoIHx8IHZhci0+eXJlc192aXJ0 dWFsICE9IGZiLT5oZWlnaHQpIHsKPiA+PiArCWlmICgoaW5mby0+ZmxhZ3MgJiBGQklORk9fTUlT Q19VU0VSRVZFTlQpICYmCj4gPj4gKwkgICAgKHZhci0+Yml0c19wZXJfcGl4ZWwgIT0gZmItPmZv cm1hdC0+Y3BwWzBdICogOCB8fAo+ID4+ICsJICAgICB2YXItPnhyZXMgIT0gZmItPndpZHRoIHx8 IHZhci0+eXJlcyAhPSBmYi0+aGVpZ2h0IHx8Cj4gPj4gKwkgICAgIHZhci0+eHJlc192aXJ0dWFs ICE9IGZiLT53aWR0aCB8fCB2YXItPnlyZXNfdmlydHVhbCAhPSBmYi0+aGVpZ2h0KSkgewo+ID4+ ICAJCURSTV9ERUJVRygiZmIgdXNlcnNwYWNlIHJlcXVlc3RlZCB3aWR0aC9oZWlnaHQvYnBwIGRp ZmZlcmVudCB0aGFuIGN1cnJlbnQgZmIgIgo+ID4+ICAJCQkgICJyZXF1ZXN0ICVkeCVkLSVkICh2 aXJ0dWFsICVkeCVkKSA+ICVkeCVkLSVkXG4iLAo+ID4+ICAJCQkgIHZhci0+eHJlcywgdmFyLT55 cmVzLCB2YXItPmJpdHNfcGVyX3BpeGVsLAo+ID4+IC0tIAo+ID4+IDIuMTEuMAo+ID4+Cj4gPj4g X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KPiA+PiBkcmkt ZGV2ZWwgbWFpbGluZyBsaXN0Cj4gPj4gZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwo+ ID4+IGh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRl dmVsCj4gPiAKPiAKPiAKPiAtLSAKPiBFYXJ0aGxpbmcgTWljaGVsIETDpG56ZXIgICAgICAgICAg ICAgICB8ICAgICAgICAgICAgICAgaHR0cDovL3d3dy5hbWQuY29tCj4gTGlicmUgc29mdHdhcmUg ZW50aHVzaWFzdCAgICAgICAgICAgICB8ICAgICAgICAgICAgIE1lc2EgYW5kIFggZGV2ZWxvcGVy CgotLSAKVmlsbGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBs aXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1h bi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751192AbdCQKpV (ORCPT ); Fri, 17 Mar 2017 06:45:21 -0400 Received: from mga01.intel.com ([192.55.52.88]:60218 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750997AbdCQKpS (ORCPT ); Fri, 17 Mar 2017 06:45:18 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,176,1486454400"; d="scan'208";a="1143687825" Date: Fri, 17 Mar 2017 12:01:33 +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, Stefan Agner Subject: Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set Message-ID: <20170317100133.GO31595@intel.com> References: <20170316095553.1586-1-michel@daenzer.net> <20170316100900.GC31595@intel.com> <87d59360-8419-b547-7b6b-32afedcf1330@daenzer.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87d59360-8419-b547-7b6b-32afedcf1330@daenzer.net> 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 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 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, so I'm not even sure what kind of values we stuff into xres & co. > > Does that make sense? Stefan, would this break any test case you wrote > your patch for? > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index f6d4d9700734..f4f70ce24fcc 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1260,8 +1260,8 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > * to KMS, hence fail if different settings are requested. > */ > if (var->bits_per_pixel != fb->format->cpp[0] * 8 || > - var->xres != fb->width || var->yres != fb->height || > - var->xres_virtual != fb->width || var->yres_virtual != fb->height) { > + var->xres > fb->width || var->yres > fb->height || > + var->xres_virtual > fb->width || var->yres_virtual > fb->height) { > DRM_DEBUG("fb userspace requested width/height/bpp different than current fb " > "request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n", > var->xres, var->yres, var->bits_per_pixel, > > > > >> Bugzilla: https://bugs.freedesktop.org/99841 > >> Fixes: 865afb11949e ("drm/fb-helper: reject any changes to the fbdev") > >> Signed-off-by: Michel Dänzer > >> --- > >> > >> I'm not entirely sure why the values can not match for a VT switch. If > >> anybody thinks this just papers over the real issue, please speak up. > >> > >> drivers/gpu/drm/drm_fb_helper.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > >> index f6d4d9700734..9663f3b8f287 100644 > >> --- a/drivers/gpu/drm/drm_fb_helper.c > >> +++ b/drivers/gpu/drm/drm_fb_helper.c > >> @@ -1259,9 +1259,10 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > >> * Changes struct fb_var_screeninfo are currently not pushed back > >> * to KMS, hence fail if different settings are requested. > >> */ > >> - if (var->bits_per_pixel != fb->format->cpp[0] * 8 || > >> - var->xres != fb->width || var->yres != fb->height || > >> - var->xres_virtual != fb->width || var->yres_virtual != fb->height) { > >> + if ((info->flags & FBINFO_MISC_USEREVENT) && > >> + (var->bits_per_pixel != fb->format->cpp[0] * 8 || > >> + var->xres != fb->width || var->yres != fb->height || > >> + var->xres_virtual != fb->width || var->yres_virtual != fb->height)) { > >> DRM_DEBUG("fb userspace requested width/height/bpp different than current fb " > >> "request %dx%d-%d (virtual %dx%d) > %dx%d-%d\n", > >> var->xres, var->yres, var->bits_per_pixel, > >> -- > >> 2.11.0 > >> > >> _______________________________________________ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer -- Ville Syrjälä Intel OTC