On Mon, 08 Nov 2021, Javier Martinez Canillas <javierm@xxxxxxxxxx> wrote: > Hello Thomas, > > On 11/8/21 13:50, Thomas Zimmermann wrote: >> Hi >> >> Am 08.11.21 um 13:15 schrieb Javier Martinez Canillas: >>> There is a lot of historical baggage on this parameter. It is defined in >>> the vgacon driver as nomodeset, but its set function is called text_mode() >>> and the value queried with a function named vgacon_text_force(). >>> >>> All this implies that it's about forcing text mode for VGA, yet it is not >>> used in neither vgacon nor other console driver. The only users for these >>> are DRM drivers, that check for the vgacon_text_force() return value to >>> determine whether the driver should be loaded or not. >>> >>> That makes it quite confusing to read the code, because the variables and >>> function names don't reflect what they actually do and also are not in the >>> same subsystem as the drivers that make use of them. >>> >>> This patch-set attempts to cleanup the code by moving the nomodseset param >>> to the DRM subsystem and do some renaming to make their intention clearer. >>> >>> This is a v3 of the patches, that address issues pointed out by Jani Nikula >>> in v2: https://lkml.org/lkml/2021/11/4/594 >>> >>> Patch #1 and #2 are just trivial cleanups. >>> >>> Patch #3 moves the nomodeset boot option to the DRM subsystem and renames >>> the variables and functions names. >>> >>> Patch #4 removes the relationship between the nomodeset parameter and the >>> CONFIG_VGA_CONSOLE Kconfig symbol. >> >> On patches 1 to 4 >> >> Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx> >> > > Thanks! > >>> >>> Patch #5 adds nomodeset to the kernel parameters documentation. >>> >>> Patch #6 improves the message when nomodeset is enabled to make it more >>> accurate and less sensational. >> >> See my comments on these patches. >> > > Yes, agreed with your feedback on these. I'll improve it when posting a v4. With the changes proposed by Thomas, the series is also Acked-by: Jani Nikula <jani.nikula@xxxxxxxxx> In particular, it's fine to merge the i915 parts via whichever tree suits you all best (I presume it's drm-misc). I might have bikeshedded the drm_get_modeset() name and the choice of drm_mode_config.h to place the declaration... but meh. The series is definitely an improvement to the status quo. BR, Jani. > > Best regards, -- > Javier Martinez Canillas > Linux Engineering > Red Hat > -- Jani Nikula, Intel Open Source Graphics Center
