[PATCH 2/2] drm/amdgpu: Add modeset module option

[Michel Dänzer]

On 2018-04-03 04:06 PM, Ville Syrjälä wrote:
> On Tue, Apr 03, 2018 at 03:47:57PM +0200, Michel Dänzer wrote:
>> On 2018-04-03 03:39 PM, Ilia Mirkin wrote:
>>> On Tue, Apr 3, 2018 at 9:32 AM, Michel Dänzer <michel at daenzer.net> wrote:
>>>> On 2018-04-03 03:26 PM, Ilia Mirkin wrote:
>>>>> On Tue, Apr 3, 2018 at 5:29 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>>>>>> On Sun, Apr 01, 2018 at 10:12:10PM +0200, Christian König wrote:
>>>>>>> Am 01.04.2018 um 20:21 schrieb Takashi Iwai:
>>>>>>>> On Sun, 01 Apr 2018 19:58:11 +0200,
>>>>>>>> Christian K6nig wrote:
>>>>>>>>> Am 01.04.2018 um 19:45 schrieb Ilia Mirkin:
>>>>>>>>>> On Sun, Apr 1, 2018 at 1:39 PM, Christian König
>>>>>>>>>> <christian.koenig at amd.com> wrote:
>>>>>>>>>>> Am 30.03.2018 um 22:45 schrieb Takashi Iwai:
>>>>>>>>>>>> amdgpu driver lacks of modeset module option other drm drivers provide
>>>>>>>>>>>> for enforcing or disabling the driver load.  Interestingly, the
>>>>>>>>>>>> amdgpu_mode variable declaration is already found in the header file,
>>>>>>>>>>>> but the actual implementation seems to have been forgotten.
>>>>>>>>>>>>
>>>>>>>>>>>> This patch adds the missing piece.
>>>>>>>>>>> NAK, modesetting is mandatory for amdgpu and we should probably remove the
>>>>>>>>>>> option to disable it from other DRM drivers without UMS support as well
>>>>>>>>>>> (pretty much all of them now).
>>>>>>>>>>>
>>>>>>>>>>> If you want to prevent a driver from loading I think the correct way to do
>>>>>>>>>>> so is to give modprobe.blacklist=amdgpu on the kernel commandline.
>>>>>>>>>>>
>>>>>>>>>>> That would remove the possibility to prevent the driver from loading when it
>>>>>>>>>>> is compiled in, but I don't see much of a problem with that.
>>>>>>>>>> Having a way to kill the graphics driver is a very useful debugging
>>>>>>>>>> tool, and also a quick and easy way to get out of an unpleasant
>>>>>>>>>> situation where graphics are messed up / system hangs / etc. The
>>>>>>>>>> modprobe blacklist kernel arg only works in certain environments (and
>>>>>>>>>> only if it's a module).
>>>>>>>>>>
>>>>>>>>>> Every other DRM driver has this and this is a well-documented
>>>>>>>>>> workaround for "graphics are messed up when I install linux", why not
>>>>>>>>>> allow a uniform experience for the end users who are just trying to
>>>>>>>>>> get their systems up and running?
>>>>>>>>> Because it is not well documented and repeated over and over again in
>>>>>>>>> drivers.
>>>>>>>>>
>>>>>>>>> The problem is that people don't realized that the driver isn't loaded
>>>>>>>>> at all without the modeset=0 module option and demand fixing the
>>>>>>>>> resulting bad performance without modesetting.
>>>>>>>> Hm, I don't get it.  What this options has to do with performance for
>>>>>>>> a KMS-only driver...?
>>>>>>>
>>>>>>> Well exactly that's the point, nothing.
>>>>>>>
>>>>>>> The problem is that the option name is confusing to the end user because the
>>>>>>> expectation is that "nomodeset" just means that they only can't change the
>>>>>>> display mode.
>>>>>>>
>>>>>>> Some (unfortunately quite a lot) people don't realize that for KMS drivers
>>>>>>> this means that the driver isn't even loaded and they also don't get any
>>>>>>> acceleration.
>>>>>>>
>>>>>>> We had to explain that numerous times now. I think it would be best to give
>>>>>>> the option a more meaningful name.
>>>>>>
>>>>>> Yeah, agreed with Christian. If we want a generic "pls disable all gfx
>>>>>> accel" knob then probably best to put that into the drm core. And just
>>>>>> outright fail loading the drm core if that happens, which will prevent all
>>>>>> gfx drivers from loading.
>>>>>>
>>>>>> That likely means a hole bunch of stuff won't work (usually sound keels
>>>>>> over too), but that's what you get for this. Only disabling modesetting
>>>>>> without disabling the entire driver doesn't work (never has, except for
>>>>>> this UMS+GEM combo only i915 support, and not for long).
>>>>>>
>>>>>> And once we have that knob, probably best to phase out all the per-driver
>>>>>> options.
>>>>>
>>>>> Another use-case that the per-driver disables enable is "i915 works
>>>>> but nouveau is broken due to crazy ACPI / PCIe PM / whatever". It
>>>>> seems likely this could happen with amdgpu as well.
>>>>
>>>> modprobe.blacklist=amdgpu
>>>>
>>>> works as well as the modeset parameter for this.
>>>
>>> People who build their own kernels run into trouble too.
>>
>> There have always been various more or less serious issues with building
>> amdgpu (and radeon) into the kernel. People who do so get to keep all
>> pieces when it breaks.
>>
>>
>>> Also does this work uniformly across all systems where it is a module?
>>
>> AFAIK yes.
> 
> Sadly modprobe.blacklist doesn't prevent X/whatever from loading the
> module anyway.

AFAICT xf86-video-amdgpu and modesetting have never even tried loading
the kernel module, and neither has xf86-video-ati for years at least.


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