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 Content-Length: 4583 Lines: 102 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