On Fri, Oct 22, 2021 at 11:16 AM Javier Martinez Canillas <javierm@xxxxxxxxxx> wrote: > > Hello Neal, > > Thanks for your feedback. > > On 10/22/21 16:56, Neal Gompa wrote: > > On Fri, Oct 22, 2021 at 10:40 AM Javier Martinez Canillas > > <javierm@xxxxxxxxxx> wrote: > >> > >> The simpledrm driver allows to use the frame buffer that was set-up by the > >> firmware. This gives early video output before the platform DRM driver is > >> probed and takes over. > >> > >> But it would be useful to have a way to disable this take over by the real > >> DRM drivers. For example, there may be bugs in the DRM drivers that could > >> cause the display output to not work correctly. > >> > >> For those cases, it would be good to keep the simpledrm driver instead and > >> at least get a working display as set-up by the firmware. > >> > >> Let's add a drm.remove_fb boolean kernel command line parameter, that when > >> set to false will prevent the conflicting framebuffers to being removed. > >> > >> Since the drivers call drm_aperture_remove_conflicting_framebuffers() very > >> early in their probe callback, this will cause the drivers' probe to fail. > >> > >> Thanks to Neal Gompa for the suggestion and Thomas Zimmermann for the idea > >> on how this could be implemented. > >> > >> Suggested-by: Neal Gompa <ngompa13@xxxxxxxxx> > >> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> > >> --- > >> Hello, > >> > >> I'm sending this as an RFC because I wasn't sure about the correct name for > >> this module parameter, and also if 'remove_fb=0' is intitutive or instead a > >> parameter that's enabled is preferred (i.e: 'disable_fb_removal=1'). > >> > > > > In general, I think the patch is fine, but it might make sense to name > > the parameter after the *effect* rather than the *action*. That is, > > the effect of this option is that we don't probe and hand over to a > > more appropriate hardware DRM driver. > > > > Since the effect (in DRM terms) is that we don't use platform DRM > > modules, perhaps we could name the option one of these: > > > > * drm.noplatformdrv > > * drm.simpledrv > > * drm.force_simple > > > > or maybe drm.disable_handover ? Naming is hard... > That would make sense for a parameter named by the action. If we want to go that route, then I'd be fine with that. My goal is to have something people understand. > > I'm inclined to say we should use the drm.* namespace for the cmdline > > option because that makes it clear what subsystem it affects. The > > legacy "nomodeset" option kind of sucked because it didn't really tell > > you what that meant, and I'd rather not repeat that mistake. > > > > Yes, agreed. In fact, that is the case for this patch since the param is > in the drm module. I just forgot to mention the namespace in the last > paragraph of the comment. > Good to know. :) -- 真実はいつも一つ！/ Always, there's only one truth!
