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

[Michel Dänzer]

On 2018-04-03 11:44 AM, Takashi Iwai wrote:
> On Tue, 03 Apr 2018 11:18:34 +0200,
> Michel D4nzer wrote:
>>
>> On 2018-04-03 11:02 AM, Takashi Iwai wrote:
>>> On Tue, 03 Apr 2018 10:57:56 +0200,
>>> Christian K6nig wrote:
>>>>
>>>> Am 03.04.2018 um 10:36 schrieb Michel Dänzer:
>>>>> On 2018-04-01 07:45 PM, Ilia Mirkin wrote:
>>>>>> 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).
>>>>> Building amdgpu into the kernel isn't feasible for a generic kernel such
>>>>> as a distro one, because it would require including all microcode into
>>>>> the kernel as well (12M right now, and growing).
>>>>>
>>>>> If a user decides to build amdgpu into their custom kernel and runs into
>>>>> trouble due to that, that's "doctor, it hurts if I do this" territory.
>>>>
>>>> Correct, but I agree that even in this situation it would be very
>>>> helpful to prevent the gfx drivers from loading and fallback to
>>>> efifb/vesafd (or whatever the platform provides).
>>>>
>>>> It's just that the "nomodeset" and "amdgpu.modeset=0" options are
>>>> really not well named for this task.
>>>
>>> Agreed with the naming mess.  But OTOH, it's already a thing that is
>>> too popular to kill.  You can add a more suitable option name, but you
>>> cannot drop these existing ones easily.  It's already in a gray zone
>>> of the golden "don't break user-space" rule.
>>
>> That's quite a stretch argument, given that amdgpu has never supported
>> the modeset parameter.
> 
> Oh I don't mean about my own patch but the foreseen action Christian
> mentioned.
> 
>> Also, module parameters aren't UAPI.
> 
> Right, but we care not only about UAPI.  If the kernel breaks
> something, it's a regression.  It's what Linus suggested many times.
> The same argument has been applied to proc or sysfs files in the past,
> and we had to correct back again.
> 
> Don't get me wrong: I'm not always objecting to the removal of any
> module existing options.  But if it break a major usage pattern, it's
> a problem.  And the removal of nomodeset option would be a kind of
> such things, IMO, that's why I mentioned as "a gray zone" in the
> above.

Note that nomodeset and the per-driver modeset parameters are separate
things. amdgpu has always supported the former, but has never supported
the latter.


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