| Message ID | 20230124134010.30263-1-tzimmermann@suse.de |
|---|---|
| Headers | show |
| Series | drm/fb-helper: Various cleanups | expand |
Hello Thomas, On 1/24/23 14:40, 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> but I've a question below. > drivers/gpu/drm/drm_client.c | 5 +++++ > drivers/gpu/drm/drm_fbdev_generic.c | 5 ----- > 2 files changed, 5 insertions(+), 5 deletions(-) [...] > --- 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; > - } > - What about the following code snippet: if (!drm_drv_uses_atomic_modeset(dev)) drm_helper_disable_unused_functions(dev); that seems to be something that should be in the core client dev hotplug as well, since it isn't specific to the fbdev emulation client ?
On 1/24/23 14:40, 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, 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 <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
On 1/24/23 14:40, Thomas Zimmermann wrote: > Initialize the fb-helper's preferred_bpp field early from within > drm_fb_helper_prepare(); instead of the later client hot-plugging > callback. This simplifies the generic fbdev setup function. > > No real changes, but all drivers' fbdev code has to be adapted. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
On 1/24/23 14:40, Thomas Zimmermann wrote: > The fbdev framebuffer cleanup in drm_fbdev_fb_destroy() calls > drm_fbdev_release() and drm_fbdev_cleanup(). Inline both into the > caller. No functional changes. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Am 25.01.23 um 09:30 schrieb Javier Martinez Canillas: > Hello Thomas, > > On 1/24/23 14:40, 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> > > but I've a question below. > >> drivers/gpu/drm/drm_client.c | 5 +++++ >> drivers/gpu/drm/drm_fbdev_generic.c | 5 ----- >> 2 files changed, 5 insertions(+), 5 deletions(-) > > [...] > >> --- 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; >> - } >> - > > What about the following code snippet: > > if (!drm_drv_uses_atomic_modeset(dev)) > drm_helper_disable_unused_functions(dev); > > that seems to be something that should be in the core client dev hotplug > as well, since it isn't specific to the fbdev emulation client ? That's in the middle of the initial probing code and disables pipeline elements in non-atomic modesetting. TBH I don't dare to move it around. If we ever have other clients, we can attempt the put it into the client. Best regards Thomas >
Add various cleanups and changes to DRM's fbdev helpers and the generic fbdev emulation. There's no clear theme here, just lots of small things that need to be updated. In the end, the code will better reflect which parts are in the DRM client, which is fbdev emulation, and which are shared fbdev helpers. v2: * cleanups in drm_fbdev_fb_destroy() (Sam) * fix declaration of drm_fb_helper_unprepare() Thomas Zimmermann (10): drm/client: Test for connectors before sending hotplug event drm/client: Add hotplug_failed flag drm/fb-helper: Introduce drm_fb_helper_unprepare() drm/fbdev-generic: Initialize fb-helper structure in generic setup drm/fb-helper: Remove preferred_bpp parameter from fbdev internals drm/fb-helper: Initialize fb-helper's preferred BPP in prepare function drm/fbdev-generic: Minimize hotplug error handling drm/fbdev-generic: Minimize client unregistering drm/fbdev-generic: Inline clean-up helpers into drm_fbdev_fb_destroy() drm/fbdev-generic: Rename struct fb_info 'fbi' to 'info' drivers/gpu/drm/armada/armada_fbdev.c | 4 +- drivers/gpu/drm/drm_client.c | 10 ++ drivers/gpu/drm/drm_fb_helper.c | 58 ++++++--- drivers/gpu/drm/drm_fbdev_generic.c | 131 ++++++++------------- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 +- drivers/gpu/drm/gma500/framebuffer.c | 4 +- drivers/gpu/drm/i915/display/intel_fbdev.c | 11 +- drivers/gpu/drm/msm/msm_fbdev.c | 4 +- drivers/gpu/drm/omapdrm/omap_fbdev.c | 4 +- drivers/gpu/drm/radeon/radeon_fb.c | 4 +- drivers/gpu/drm/tegra/fb.c | 7 +- include/drm/drm_client.h | 8 ++ include/drm/drm_fb_helper.h | 8 +- 13 files changed, 134 insertions(+), 123 deletions(-) base-commit: 7d3e7f64a42d66ba8da6e7b66a8d85457ef84570