Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
Hi Am 27.01.23 um 19:02 schrieb Simon Ser: On Wednesday, January 25th, 2023 at 21:04, Thomas Zimmermann 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 -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
On Wednesday, January 25th, 2023 at 21:04, Thomas Zimmermann 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?
Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
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 > > > Reviewed-by: Javier Martinez Canillas > > > --- > > > 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
Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
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 Reviewed-by: Javier Martinez Canillas --- 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(>clientlist_mutex); list_for_each_entry(client, >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, _fb_helper_generic_funcs); ret = drm_fb_helper_init(dev, fb_helper); -- 2.39.0 -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
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 > Reviewed-by: Javier Martinez Canillas > --- > 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(>clientlist_mutex); > list_for_each_entry(client, >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, _fb_helper_generic_funcs); > > ret = drm_fb_helper_init(dev, fb_helper); > -- > 2.39.0
[PATCH v3 01/10] drm/client: Test for connectors before sending hotplug event
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 Reviewed-by: Javier Martinez Canillas --- 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; + } + mutex_lock(>clientlist_mutex); list_for_each_entry(client, >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, _fb_helper_generic_funcs); ret = drm_fb_helper_init(dev, fb_helper); -- 2.39.0