Re: [Intel-gfx] [PATCH v3 02/10] drm/client: Add hotplug_failed flag
Hi
Am 25.01.23 um 21:57 schrieb Sam Ravnborg:
Hi Thomas,
On Wed, Jan 25, 2023 at 09:04:07PM +0100, Thomas Zimmermann wrote:
Signal failed hotplugging with a flag in struct drm_client_dev. If set,
the client helpers will not further try to set up the fbdev display.
This used to be signalled with a combination of cleared pointers in
struct drm_fb_helper,
I failed to find where we clear the pointers. What do I miss?
Those pointer fields, dev and funcs, where allocated with kzalloc(). The
error path in drm_fbdev_client_hotplug() later reset them to NULL again
if an error occured.
Best regards
Thomas
(I had assumed we would stop clearing the pointers after this change).
Sam
which prevents us from initializing these pointers
early after allocation.
The change also harmonizes behavior among DRM clients. Additional DRM
clients will now handle failed hotplugging like fbdev does.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Javier Martinez Canillas
---
drivers/gpu/drm/drm_client.c| 5 +
drivers/gpu/drm/drm_fbdev_generic.c | 4
include/drm/drm_client.h| 8
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 09ac191c202d..009e7b10455c 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -208,8 +208,13 @@ void drm_client_dev_hotplug(struct drm_device *dev)
if (!client->funcs || !client->funcs->hotplug)
continue;
+ if (client->hotplug_failed)
+ continue;
+
ret = client->funcs->hotplug(client);
drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
+ if (ret)
+ client->hotplug_failed = true;
}
mutex_unlock(>clientlist_mutex);
}
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c
b/drivers/gpu/drm/drm_fbdev_generic.c
index 3d455a2e3fb5..135d58b8007b 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -382,10 +382,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev
*client)
struct drm_device *dev = client->dev;
int ret;
- /* Setup is not retried if it has failed */
- if (!fb_helper->dev && fb_helper->funcs)
- return 0;
-
if (dev->fb_helper)
return drm_fb_helper_hotplug_event(dev->fb_helper);
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 4fc8018eddda..39482527a775 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -106,6 +106,14 @@ struct drm_client_dev {
* @modesets: CRTC configurations
*/
struct drm_mode_set *modesets;
+
+ /**
+* @hotplug failed:
+*
+* Set by client hotplug helpers if the hotplugging failed
+* before. It is usually not tried again.
+*/
+ bool hotplug_failed;
};
int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
--
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: [Intel-gfx] [PATCH v3 02/10] drm/client: Add hotplug_failed flag
Hi Thomas,
On Wed, Jan 25, 2023 at 09:04:07PM +0100, Thomas Zimmermann wrote:
> Signal failed hotplugging with a flag in struct drm_client_dev. If set,
> the client helpers will not further try to set up the fbdev display.
>
> This used to be signalled with a combination of cleared pointers in
> struct drm_fb_helper,
I failed to find where we clear the pointers. What do I miss?
(I had assumed we would stop clearing the pointers after this change).
Sam
which prevents us from initializing these pointers
> early after allocation.
>
> The change also harmonizes behavior among DRM clients. Additional DRM
> clients will now handle failed hotplugging like fbdev does.
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Javier Martinez Canillas
> ---
> drivers/gpu/drm/drm_client.c| 5 +
> drivers/gpu/drm/drm_fbdev_generic.c | 4
> include/drm/drm_client.h| 8
> 3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 09ac191c202d..009e7b10455c 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -208,8 +208,13 @@ void drm_client_dev_hotplug(struct drm_device *dev)
> if (!client->funcs || !client->funcs->hotplug)
> continue;
>
> + if (client->hotplug_failed)
> + continue;
> +
> ret = client->funcs->hotplug(client);
> drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
> + if (ret)
> + client->hotplug_failed = true;
> }
> mutex_unlock(>clientlist_mutex);
> }
> diff --git a/drivers/gpu/drm/drm_fbdev_generic.c
> b/drivers/gpu/drm/drm_fbdev_generic.c
> index 3d455a2e3fb5..135d58b8007b 100644
> --- a/drivers/gpu/drm/drm_fbdev_generic.c
> +++ b/drivers/gpu/drm/drm_fbdev_generic.c
> @@ -382,10 +382,6 @@ static int drm_fbdev_client_hotplug(struct
> drm_client_dev *client)
> struct drm_device *dev = client->dev;
> int ret;
>
> - /* Setup is not retried if it has failed */
> - if (!fb_helper->dev && fb_helper->funcs)
> - return 0;
> -
> if (dev->fb_helper)
> return drm_fb_helper_hotplug_event(dev->fb_helper);
>
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 4fc8018eddda..39482527a775 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -106,6 +106,14 @@ struct drm_client_dev {
>* @modesets: CRTC configurations
>*/
> struct drm_mode_set *modesets;
> +
> + /**
> + * @hotplug failed:
> + *
> + * Set by client hotplug helpers if the hotplugging failed
> + * before. It is usually not tried again.
> + */
> + bool hotplug_failed;
> };
>
> int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
> --
> 2.39.0
[Intel-gfx] [PATCH v3 02/10] drm/client: Add hotplug_failed flag
Signal failed hotplugging with a flag in struct drm_client_dev. If set,
the client helpers will not further try to set up the fbdev display.
This used to be signalled with a combination of cleared pointers in
struct drm_fb_helper, which prevents us from initializing these pointers
early after allocation.
The change also harmonizes behavior among DRM clients. Additional DRM
clients will now handle failed hotplugging like fbdev does.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Javier Martinez Canillas
---
drivers/gpu/drm/drm_client.c| 5 +
drivers/gpu/drm/drm_fbdev_generic.c | 4
include/drm/drm_client.h| 8
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index 09ac191c202d..009e7b10455c 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -208,8 +208,13 @@ void drm_client_dev_hotplug(struct drm_device *dev)
if (!client->funcs || !client->funcs->hotplug)
continue;
+ if (client->hotplug_failed)
+ continue;
+
ret = client->funcs->hotplug(client);
drm_dbg_kms(dev, "%s: ret=%d\n", client->name, ret);
+ if (ret)
+ client->hotplug_failed = true;
}
mutex_unlock(>clientlist_mutex);
}
diff --git a/drivers/gpu/drm/drm_fbdev_generic.c
b/drivers/gpu/drm/drm_fbdev_generic.c
index 3d455a2e3fb5..135d58b8007b 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -382,10 +382,6 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev
*client)
struct drm_device *dev = client->dev;
int ret;
- /* Setup is not retried if it has failed */
- if (!fb_helper->dev && fb_helper->funcs)
- return 0;
-
if (dev->fb_helper)
return drm_fb_helper_hotplug_event(dev->fb_helper);
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 4fc8018eddda..39482527a775 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -106,6 +106,14 @@ struct drm_client_dev {
* @modesets: CRTC configurations
*/
struct drm_mode_set *modesets;
+
+ /**
+* @hotplug failed:
+*
+* Set by client hotplug helpers if the hotplugging failed
+* before. It is usually not tried again.
+*/
+ bool hotplug_failed;
};
int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
--
2.39.0
