Messages in this thread | | | Date | Tue, 11 Jul 2023 00:11:50 +0300 | Subject | Re: [PATCH] drm/client: Send hotplug event after registering a client | From | Dmitry Baryshkov <> |
| |
On 10/07/2023 12:10, Thomas Zimmermann wrote: > Generate a hotplug event after registering a client to allow the > client to configure its display. Remove the hotplug calls from the > existing clients for fbdev emulation. This change fixes a concurrency > bug between registering a client and receiving events from the DRM > core. The bug is present in the fbdev emulation of all drivers. > > The fbdev emulation currently generates a hotplug event before > registering the client to the device. For each new output, the DRM > core sends an additional hotplug event to each registered client. > > If the DRM core detects first output between sending the artificial > hotplug and registering the device, the output's hotplug event gets > lost. If this is the first output, the fbdev console display remains > dark. This has been observed with amdgpu and fbdev-generic. > > Fix this by adding hotplug generation directly to the client's > register helper drm_client_register(). Registering the client and > receiving events are serialized by struct drm_device.clientlist_mutex. > So an output is either configured by the initial hotplug event, or > the client has already been registered. > > The bug was originally added in commit 6e3f17ee73f7 ("drm/fb-helper: > generic: Call drm_client_add() after setup is done"), in which adding > a client and receiving a hotplug event switched order. It was hidden, > as most hardware and drivers have at least on static output configured. > Other drivers didn't use the internal DRM client or still had struct > drm_mode_config_funcs.output_poll_changed set. That callback handled > hotplug events as well. After not setting the callback in amdgpu in > commit 0e3172bac3f4 ("drm/amdgpu: Don't set struct > drm_driver.output_poll_changed"), amdgpu did not show a framebuffer > console if output events got lost. The bug got copy-pasted from > fbdev-generic into the other fbdev emulation. > > Reported-by: Moritz Duge <MoritzDuge@kolahilft.de> > Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2649 > Fixes: 6e3f17ee73f7 ("drm/fb-helper: generic: Call drm_client_add() after setup is done") > Fixes: 8ab59da26bc0 ("drm/fb-helper: Move generic fbdev emulation into separate source file") > Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for GEM DMA helpers") > Fixes: 63c381552f69 ("drm/armada: Implement fbdev emulation as in-kernel client") > Fixes: 49953b70e7d3 ("drm/exynos: Implement fbdev emulation as in-kernel client") > Fixes: 8f1aaccb04b7 ("drm/gma500: Implement client-based fbdev emulation") > Fixes: 940b869c2f2f ("drm/msm: Implement fbdev emulation as in-kernel client") > Fixes: 9e69bcd88e45 ("drm/omapdrm: Implement fbdev emulation as in-kernel client") > Fixes: e317a69fe891 ("drm/radeon: Implement client-based fbdev emulation") > Fixes: 71ec16f45ef8 ("drm/tegra: Implement fbdev emulation as in-kernel client") > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > Tested-by: Moritz Duge <MoritzDuge@kolahilft.de> > Tested-by: Torsten Krah <krah.tm@gmail.com> > Tested-by: Paul Schyska <pschyska@gmail.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: David Airlie <airlied@gmail.com> > Cc: Noralf Trønnes <noralf@tronnes.org> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Javier Martinez Canillas <javierm@redhat.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: Inki Dae <inki.dae@samsung.com> > Cc: Seung-Woo Kim <sw0312.kim@samsung.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > Cc: Rob Clark <robdclark@gmail.com> > Cc: Abhinav Kumar <quic_abhinavk@quicinc.com> > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: "Christian König" <christian.koenig@amd.com> > Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Mikko Perttunen <mperttunen@nvidia.com> > Cc: dri-devel@lists.freedesktop.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-samsung-soc@vger.kernel.org > Cc: linux-arm-msm@vger.kernel.org > Cc: freedreno@lists.freedesktop.org > Cc: amd-gfx@lists.freedesktop.org > Cc: linux-tegra@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: <stable@vger.kernel.org> # v5.2+ > --- > drivers/gpu/drm/armada/armada_fbdev.c | 4 ---- > drivers/gpu/drm/drm_client.c | 21 +++++++++++++++++++++ > drivers/gpu/drm/drm_fbdev_dma.c | 4 ---- > drivers/gpu/drm/drm_fbdev_generic.c | 4 ---- > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 4 ---- > drivers/gpu/drm/gma500/fbdev.c | 4 ---- > drivers/gpu/drm/msm/msm_fbdev.c | 4 ----
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> # msm
> drivers/gpu/drm/omapdrm/omap_fbdev.c | 4 ---- > drivers/gpu/drm/radeon/radeon_fbdev.c | 4 ---- > drivers/gpu/drm/tegra/fbdev.c | 4 ---- > 10 files changed, 21 insertions(+), 36 deletions(-)
BTW: As you have been clearing this area. I see that significant amount of DRM drivers use exactly the same code for msm_fbdev_client_funcs and for the significant part of foo_fbdev_setup(). Do you have any plans for moving that into a library / generic code? If not, I can take a look at crafting the patch.
-- With best wishes Dmitry
| |