Hi,
On 2024/8/20 15:39, Thomas Zimmermann wrote:
Hi
Am 19.08.24 um 10:52 schrieb Sui Jingfeng:
Hi, Thomas
I love your patch, yet ...
On 2024/8/16 20:23, Thomas Zimmermann wrote:
i915's fbdev contains additional code for hotplugging a display that
cannot be ported to the common fbdev client. Introduce the callback
struct drm_fb_helper.fb_hotplug and implement it for i915. The fbdev
helpers invoke the callback before handing the hotplug event.
Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tursulin@xxxxxxxxxxx>
Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Cc: "Thomas Hellström" <thomas.hellstrom@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/drm_fb_helper.c | 6 +++
drivers/gpu/drm/i915/display/intel_fbdev.c | 43
++++++++++++----------
include/drm/drm_fb_helper.h | 13 +++++++
3 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c
b/drivers/gpu/drm/drm_fb_helper.c
index d9e539b0fd1a..92926cb02dfb 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1938,6 +1938,12 @@ int drm_fb_helper_hotplug_event(struct
drm_fb_helper *fb_helper)
if (!drm_fbdev_emulation || !fb_helper)
return 0;
+ if (fb_helper->funcs->fb_hotplug) {
We seems need to check the existence on the 'fb_helper->funcs' here,
For example:
if (fb_helper->funcs && fb_helper->funcs->fb_hotplug) {
Otherwise, it will de-reference NULL pointer.
Can be observed on a trivial driver though,
with no monitor(display) connected.
Indeed. That needs to be fixed. Thank you for noting.
Thanks for you efforts then.
To give some context: I was hoping to remove drm_fb_helper_funcs at
some point.
Yeah, too many helper functions may make peoples daze.
fb_probe is now gone with these patches and fb_dirty can certainly be
replaced as well. (I once had prototype patches to do that).
Well, the grammar of "ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);" looks strange,
It's lengthy and I observed you have cleaned it up at the last patch.
Which also eliminates one pair "if and else" clause, the codes looks
more fluent now.
This leaves the new callbacks for 915, for which I don't have a good
alternative solution. So it seems that drm_fb_helper_funcs will only
be used by i915/xe in the long term.
Well, since it is a DRM client now, maybe we could try to drop it into struct drm_driver.
Just like the '.fbdev_probe' callback, this may help to achieve a 100% DRM-based console/logger IMO.
Besides, a lot of DRM driver instances has the DMA/2D acceleration hardware, promote it
into drm_driver structure may has the potential to utilize hardware acceleration. Drivers
will more easily to have custom implementation. I'm not 100% sure if it will only be used
by i915 in the future.
Best regards,
Sui
