lkml.org 
[lkml]   [2023]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] drm/client: Send hotplug event after registering a client
    From
    +regressions
    On 7/10/2023 04:58, Thomas Zimmermann wrote:
    > Hi
    >
    > Am 10.07.23 um 11:52 schrieb Javier Martinez Canillas:
    >> Thomas Zimmermann <tzimmermann@suse.de> writes:
    >>
    >> Hello Thomas,
    >>
    >>> 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
    >>
    >> Aren't you missing a Fixes: for 0e3172bac3f4 too? Since that's the commit
    >> that unmasked the bug for amdgpu, IMO that is the most important to list.
    >
    > Well, OK.
    >
    >>
    >>> 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+
    >>
    >> While it's true that the but was introduced by commit 6e3f17ee73f7 and
    >> that
    >> landed in v5.2, I wonder if this patch could even be applied to such
    >> olders
    >> Linux versions. Probably in practice it would be at most backported to
    >> v6.2, which is the release that exposed the bug for the amdgpu driver.
    >
    > No idea. The fix looks simple enough, but a lot has changed in the
    > surrounding code.
    >

    Actually it needs to go to at least 6.1.y.

    Moritz found it in 6.1.35 (not present in 6.1.34).


    > Best regards
    > Thomas
    >
    >>
    >> Your explanation makes sense to me and the patch looks good.
    >>
    >> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
    >>
    >

    \
     
     \ /
      Last update: 2023-07-10 18:58    [W:5.468 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site