| Message ID | 20211215010008.2545520-8-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drm: Make all drivers to honour the nomodeset parameter | expand |
Hi Javier, Am Mittwoch, dem 15.12.2021 um 01:59 +0100 schrieb Javier Martinez Canillas: > According to disable Documentation/admin-guide/kernel-parameters.txt, this > parameter can be used to disable kernel modesetting. > > DRM drivers will not perform display-mode changes or accelerated rendering > and only the systewm system framebuffer will be available if it was set-up. > Etnaviv is a render-only driver, so will no perform any modesetting on a display device, so I'm not sure if it's sensible to cover it under the nomodeset parameter. I see that it is consistent with the other drivers that deal with a combined render/display device, where the render device also gets disabled with the nomodeset param, but it doesn't really match the description of what the parameter is supposed to do. I'm not opposed to take this patch for consistency reasons, but I would like to hear some more opinions from other DRM folks. Regards, Lucas > But only a few DRM drivers currently check for nomodeset, make this driver > to also support the command line parameter. > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > --- > > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index 7dcc6392792d..58b092248f7b 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -635,6 +635,9 @@ static int __init etnaviv_init(void) > int ret; > struct device_node *np; > > + if (drm_firmware_drivers_only()) > + return -ENODEV; > + > etnaviv_validate_init(); > > ret = platform_driver_register(&etnaviv_gpu_driver);
(cc'ing Maxime) Hi Am 15.12.21 um 10:18 schrieb Lucas Stach: > Hi Javier, > > Am Mittwoch, dem 15.12.2021 um 01:59 +0100 schrieb Javier Martinez Canillas: >> According to disable Documentation/admin-guide/kernel-parameters.txt, this >> parameter can be used to disable kernel modesetting. >> >> DRM drivers will not perform display-mode changes or accelerated rendering >> and only the systewm system framebuffer will be available if it was set-up. >> > Etnaviv is a render-only driver, so will no perform any modesetting on > a display device, so I'm not sure if it's sensible to cover it under > the nomodeset parameter. I see that it is consistent with the other > drivers that deal with a combined render/display device, where the > render device also gets disabled with the nomodeset param, but it > doesn't really match the description of what the parameter is supposed > to do. > > I'm not opposed to take this patch for consistency reasons, but I would > like to hear some more opinions from other DRM folks. Our assumption is that we want to disable all DRM drivers; except those that operate on the firmware's original framebuffer. That's why the the test is called drm_firmware_drivers_only(). We know that nomodeset is a terrible name. We only kept it because it was already there, widely used, and already does what we need. We had similar concerns with the v3d driver of vc4. Javier, maybe we should leave-out such special cases for now and discuss them separately? Best regards Thomas > > Regards, > Lucas > >> But only a few DRM drivers currently check for nomodeset, make this driver >> to also support the command line parameter. >> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> >> --- >> >> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> index 7dcc6392792d..58b092248f7b 100644 >> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c >> @@ -635,6 +635,9 @@ static int __init etnaviv_init(void) >> int ret; >> struct device_node *np; >> >> + if (drm_firmware_drivers_only()) >> + return -ENODEV; >> + >> etnaviv_validate_init(); >> >> ret = platform_driver_register(&etnaviv_gpu_driver); > >
[adding Erico Nunes to Cc list] On 12/15/21 10:39, Thomas Zimmermann wrote: > (cc'ing Maxime) > > Hi > > Am 15.12.21 um 10:18 schrieb Lucas Stach: >> Hi Javier, >> >> Am Mittwoch, dem 15.12.2021 um 01:59 +0100 schrieb Javier Martinez Canillas: >>> According to disable Documentation/admin-guide/kernel-parameters.txt, this >>> parameter can be used to disable kernel modesetting. >>> >>> DRM drivers will not perform display-mode changes or accelerated rendering >>> and only the systewm system framebuffer will be available if it was set-up. >>> >> Etnaviv is a render-only driver, so will no perform any modesetting on >> a display device, so I'm not sure if it's sensible to cover it under >> the nomodeset parameter. I see that it is consistent with the other >> drivers that deal with a combined render/display device, where the >> render device also gets disabled with the nomodeset param, but it >> doesn't really match the description of what the parameter is supposed >> to do. >> >> I'm not opposed to take this patch for consistency reasons, but I would >> like to hear some more opinions from other DRM folks. > > Our assumption is that we want to disable all DRM drivers; except those > that operate on the firmware's original framebuffer. That's why the the > test is called drm_firmware_drivers_only(). > Yes, we tried to document the implicit "nomodeset" semantics to make that clear: https://cgit.freedesktop.org/drm/drm-misc/commit/?id=b22a15a5aca34c8f59b770f858b1c21d347175e0 > We know that nomodeset is a terrible name. We only kept it because it > was already there, widely used, and already does what we need. > > We had similar concerns with the v3d driver of vc4. Javier, maybe we > should leave-out such special cases for now and discuss them separately? > I was discussing the same with Erico (one of the lima driver developers). Agree that we could leave those for now. Will drop from the patch-set all the DRM drivers that don't have the DRIVER_MODESET .driver_features flag. > Best regards > Thomas > Best regards,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index 7dcc6392792d..58b092248f7b 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -635,6 +635,9 @@ static int __init etnaviv_init(void) int ret; struct device_node *np; + if (drm_firmware_drivers_only()) + return -ENODEV; + etnaviv_validate_init(); ret = platform_driver_register(&etnaviv_gpu_driver);
According to disable Documentation/admin-guide/kernel-parameters.txt, this parameter can be used to disable kernel modesetting. DRM drivers will not perform display-mode changes or accelerated rendering and only the systewm system framebuffer will be available if it was set-up. But only a few DRM drivers currently check for nomodeset, make this driver to also support the command line parameter. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> --- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++ 1 file changed, 3 insertions(+)