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

["Michel Dänzer" <michel@daenzer.net>]

On 17/03/17 08:13 PM, Ville Syrjälä wrote:
> 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 <michel.daenzer@amd.com>
>>>>>>
>>>>>> 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.

I see, thanks for clarifying.

Something like that could work, but it might still be artificially
limiting. Thinking through all the ramifications is too involved for me
right now.

Anyway, I don't think we can do anything fancier than the quasi-revert
for 4.10 or even 4.11.


>>> 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.

I'd also assume something like what you describe when fbdev emulation is
initialized, but I'm not sure there's any code to update any of that
stuff when hotplugging displays.


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