Re: [PATCH] drm/fb-helper: Only reject FB changes if FB_MISC_USER_EVENT is set

From: Michel DÃnzer
Date: Fri Mar 17 2017 - 06:45:25 EST


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 <michel.daenzer@xxxxxxx>
>>>>
>>>> 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? 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.


--
Earthling Michel DÃnzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer