| Message ID | 20230125200415.14123-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c2bb3be64eb7182285846123219230375af61abd |
| Headers | show |
| Series | drm/fb-helper: Various cleanups | expand |
Hi Thomas, On Wed, Jan 25, 2023 at 09:04:06PM +0100, Thomas Zimmermann wrote: > Test for connectors in the client code and remove a similar test > from the generic fbdev emulation. Do nothing if the test fails. > Not having connectors indicates a driver bug. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> > --- > drivers/gpu/drm/drm_client.c | 5 +++++ > drivers/gpu/drm/drm_fbdev_generic.c | 5 ----- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > index 262ec64d4397..09ac191c202d 100644 > --- a/drivers/gpu/drm/drm_client.c > +++ b/drivers/gpu/drm/drm_client.c > @@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev) > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > return; > > + if (!dev->mode_config.num_connector) { > + drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n"); > + return; This deserves a more visible logging - if a driver fails here it would be good to spot it in the normal kernel log. drm_info or drm_notice? The original code had this on the debug level, but when moving the log level could also be updated. Sam > + } > + > mutex_lock(&dev->clientlist_mutex); > list_for_each_entry(client, &dev->clientlist, list) { > if (!client->funcs || !client->funcs->hotplug) > diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c > index 0a4c160e0e58..3d455a2e3fb5 100644 > --- a/drivers/gpu/drm/drm_fbdev_generic.c > +++ b/drivers/gpu/drm/drm_fbdev_generic.c > @@ -389,11 +389,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client) > if (dev->fb_helper) > return drm_fb_helper_hotplug_event(dev->fb_helper); > > - if (!dev->mode_config.num_connector) { > - drm_dbg_kms(dev, "No connectors found, will not create framebuffer!\n"); > - return 0; > - } > - > drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs); > > ret = drm_fb_helper_init(dev, fb_helper); > -- > 2.39.0
Hi Am 25.01.23 um 21:52 schrieb Sam Ravnborg: > Hi Thomas, > > On Wed, Jan 25, 2023 at 09:04:06PM +0100, Thomas Zimmermann wrote: >> Test for connectors in the client code and remove a similar test >> from the generic fbdev emulation. Do nothing if the test fails. >> Not having connectors indicates a driver bug. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >> --- >> drivers/gpu/drm/drm_client.c | 5 +++++ >> drivers/gpu/drm/drm_fbdev_generic.c | 5 ----- >> 2 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c >> index 262ec64d4397..09ac191c202d 100644 >> --- a/drivers/gpu/drm/drm_client.c >> +++ b/drivers/gpu/drm/drm_client.c >> @@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev) >> if (!drm_core_check_feature(dev, DRIVER_MODESET)) >> return; >> >> + if (!dev->mode_config.num_connector) { >> + drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n"); >> + return; > This deserves a more visible logging - if a driver fails here it would > be good to spot it in the normal kernel log. > drm_info or drm_notice? But is that really noteworthy? AFAIK, this situation can legally happen. So if it's expected, why should we print a message about it? Best regards Thomas > > The original code had this on the debug level, but when moving the log > level could also be updated. > > Sam > >> + } >> + >> mutex_lock(&dev->clientlist_mutex); >> list_for_each_entry(client, &dev->clientlist, list) { >> if (!client->funcs || !client->funcs->hotplug) >> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c >> index 0a4c160e0e58..3d455a2e3fb5 100644 >> --- a/drivers/gpu/drm/drm_fbdev_generic.c >> +++ b/drivers/gpu/drm/drm_fbdev_generic.c >> @@ -389,11 +389,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client) >> if (dev->fb_helper) >> return drm_fb_helper_hotplug_event(dev->fb_helper); >> >> - if (!dev->mode_config.num_connector) { >> - drm_dbg_kms(dev, "No connectors found, will not create framebuffer!\n"); >> - return 0; >> - } >> - >> drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs); >> >> ret = drm_fb_helper_init(dev, fb_helper); >> -- >> 2.39.0
On Fri, Jan 27, 2023 at 03:13:50PM +0100, Thomas Zimmermann wrote: > Hi > > Am 25.01.23 um 21:52 schrieb Sam Ravnborg: > > Hi Thomas, > > > > On Wed, Jan 25, 2023 at 09:04:06PM +0100, Thomas Zimmermann wrote: > > > Test for connectors in the client code and remove a similar test > > > from the generic fbdev emulation. Do nothing if the test fails. > > > Not having connectors indicates a driver bug. > > > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> > > > --- > > > drivers/gpu/drm/drm_client.c | 5 +++++ > > > drivers/gpu/drm/drm_fbdev_generic.c | 5 ----- > > > 2 files changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > > > index 262ec64d4397..09ac191c202d 100644 > > > --- a/drivers/gpu/drm/drm_client.c > > > +++ b/drivers/gpu/drm/drm_client.c > > > @@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev) > > > if (!drm_core_check_feature(dev, DRIVER_MODESET)) > > > return; > > > + if (!dev->mode_config.num_connector) { > > > + drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n"); > > > + return; > > This deserves a more visible logging - if a driver fails here it would > > be good to spot it in the normal kernel log. > > drm_info or drm_notice? > > But is that really noteworthy? AFAIK, this situation can legally happen. So > if it's expected, why should we print a message about it? I was reading it as a driver error - as that's not the case current code is fine. Sam
On Wednesday, January 25th, 2023 at 21:04, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Not having connectors indicates a driver bug.
Is it? What if all connectors are of the DP-MST type, ie. they are
created on-the-fly?
Hi Am 27.01.23 um 19:02 schrieb Simon Ser: > On Wednesday, January 25th, 2023 at 21:04, Thomas Zimmermann <tzimmermann@suse.de> wrote: > >> Not having connectors indicates a driver bug. > > Is it? What if all connectors are of the DP-MST type, ie. they are > created on-the-fly? My commit message was nonsense. I even write this here that having no connectors is legitimate. Best regards Thomas
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 262ec64d4397..09ac191c202d 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -198,6 +198,11 @@ void drm_client_dev_hotplug(struct drm_device *dev) if (!drm_core_check_feature(dev, DRIVER_MODESET)) return; + if (!dev->mode_config.num_connector) { + drm_dbg_kms(dev, "No connectors found, will not send hotplug events!\n"); + return; + } + mutex_lock(&dev->clientlist_mutex); list_for_each_entry(client, &dev->clientlist, list) { if (!client->funcs || !client->funcs->hotplug) diff --git a/drivers/gpu/drm/drm_fbdev_generic.c b/drivers/gpu/drm/drm_fbdev_generic.c index 0a4c160e0e58..3d455a2e3fb5 100644 --- a/drivers/gpu/drm/drm_fbdev_generic.c +++ b/drivers/gpu/drm/drm_fbdev_generic.c @@ -389,11 +389,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client) if (dev->fb_helper) return drm_fb_helper_hotplug_event(dev->fb_helper); - if (!dev->mode_config.num_connector) { - drm_dbg_kms(dev, "No connectors found, will not create framebuffer!\n"); - return 0; - } - drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs); ret = drm_fb_helper_init(dev, fb_helper);