From: Sasha Levin <sashal@kernel.org> To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Sasha Levin <sashal@kernel.org>, Carsten Haitzler <carsten.haitzler@arm.com>, christian.koenig@amd.com, Liviu Dudau <liviu.dudau@arm.com>, dri-devel@lists.freedesktop.org, sumit.semwal@linaro.org, linaro-mm-sig@lists.linaro.org, james.qian.wang@arm.com, mihail.atanassov@arm.com, linux-media@vger.kernel.org Subject: [PATCH AUTOSEL 5.15 04/25] drm/komeda: Fix handling of atomic commits in the atomic_commit_tail hook Date: Sun, 9 Oct 2022 19:54:04 -0400 [thread overview] Message-ID: <20221009235426.1231313-4-sashal@kernel.org> (raw) In-Reply-To: <20221009235426.1231313-1-sashal@kernel.org> From: Liviu Dudau <liviu.dudau@arm.com> [ Upstream commit eaa225b6b52233d45457fd33730e1528c604d92d ] Komeda driver relies on the generic DRM atomic helper functions to handle commits. It only implements an atomic_commit_tail hook for the mode_config_helper_funcs and even that one is pretty close to the generic implementation with the exception of additional dma_fence signalling. What the generic helper framework doesn't do is waiting for the actual hardware to signal that the commit parameters have been written into the appropriate registers. As we signal CRTC events only on the irq handlers, we need to flush the configuration and wait for the hardware to respond. Add the Komeda specific implementation for atomic_commit_hw_done() that flushes and waits for flip done before calling drm_atomic_helper_commit_hw_done(). The fix was prompted by a patch from Carsten Haitzler where he was trying to solve the same issue but in a different way that I think can lead to wrong event signaling to userspace. Reported-by: Carsten Haitzler <carsten.haitzler@arm.com> Tested-by: Carsten Haitzler <carsten.haitzler@arm.com> Reviewed-by: Carsten Haitzler <carsten.haitzler@arm.com> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com> Link: https://patchwork.freedesktop.org/patch/msgid/20220722122139.288486-1-liviu.dudau@arm.com Signed-off-by: Sasha Levin <sashal@kernel.org> --- .../gpu/drm/arm/display/komeda/komeda_crtc.c | 4 ++-- .../gpu/drm/arm/display/komeda/komeda_kms.c | 21 ++++++++++++++++++- .../gpu/drm/arm/display/komeda/komeda_kms.h | 2 ++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c index 59172acb9738..292f533d8cf0 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c @@ -235,7 +235,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, crtc->state->event = NULL; drm_crtc_send_vblank_event(crtc, event); } else { - DRM_WARN("CRTC[%d]: FLIP happen but no pending commit.\n", + DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n", drm_crtc_index(&kcrtc->base)); } spin_unlock_irqrestore(&crtc->dev->event_lock, flags); @@ -286,7 +286,7 @@ komeda_crtc_atomic_enable(struct drm_crtc *crtc, komeda_crtc_do_flush(crtc, old); } -static void +void komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc, struct completion *input_flip_done) { diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index 93b7f09b96ca..327051bba5b6 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -69,6 +69,25 @@ static const struct drm_driver komeda_kms_driver = { .minor = 1, }; +static void komeda_kms_atomic_commit_hw_done(struct drm_atomic_state *state) +{ + struct drm_device *dev = state->dev; + struct komeda_kms_dev *kms = to_kdev(dev); + int i; + + for (i = 0; i < kms->n_crtcs; i++) { + struct komeda_crtc *kcrtc = &kms->crtcs[i]; + + if (kcrtc->base.state->active) { + struct completion *flip_done = NULL; + if (kcrtc->base.state->event) + flip_done = kcrtc->base.state->event->base.completion; + komeda_crtc_flush_and_wait_for_flip_done(kcrtc, flip_done); + } + } + drm_atomic_helper_commit_hw_done(state); +} + static void komeda_kms_commit_tail(struct drm_atomic_state *old_state) { struct drm_device *dev = old_state->dev; @@ -81,7 +100,7 @@ static void komeda_kms_commit_tail(struct drm_atomic_state *old_state) drm_atomic_helper_commit_modeset_enables(dev, old_state); - drm_atomic_helper_commit_hw_done(old_state); + komeda_kms_atomic_commit_hw_done(old_state); drm_atomic_helper_wait_for_flip_done(dev, old_state); diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h index 456f3c435719..bf6e8fba5061 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h @@ -182,6 +182,8 @@ void komeda_kms_cleanup_private_objs(struct komeda_kms_dev *kms); void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, struct komeda_events *evts); +void komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc, + struct completion *input_flip_done); struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev); void komeda_kms_detach(struct komeda_kms_dev *kms); -- 2.35.1
WARNING: multiple messages have this Message-ID (diff)
From: Sasha Levin <sashal@kernel.org> To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Liviu Dudau <liviu.dudau@arm.com>, Carsten Haitzler <carsten.haitzler@arm.com>, Sasha Levin <sashal@kernel.org>, james.qian.wang@arm.com, mihail.atanassov@arm.com, brian.starkey@arm.com, airlied@gmail.com, daniel@ffwll.ch, sumit.semwal@linaro.org, christian.koenig@amd.com, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org Subject: [PATCH AUTOSEL 5.15 04/25] drm/komeda: Fix handling of atomic commits in the atomic_commit_tail hook Date: Sun, 9 Oct 2022 19:54:04 -0400 [thread overview] Message-ID: <20221009235426.1231313-4-sashal@kernel.org> (raw) In-Reply-To: <20221009235426.1231313-1-sashal@kernel.org> From: Liviu Dudau <liviu.dudau@arm.com> [ Upstream commit eaa225b6b52233d45457fd33730e1528c604d92d ] Komeda driver relies on the generic DRM atomic helper functions to handle commits. It only implements an atomic_commit_tail hook for the mode_config_helper_funcs and even that one is pretty close to the generic implementation with the exception of additional dma_fence signalling. What the generic helper framework doesn't do is waiting for the actual hardware to signal that the commit parameters have been written into the appropriate registers. As we signal CRTC events only on the irq handlers, we need to flush the configuration and wait for the hardware to respond. Add the Komeda specific implementation for atomic_commit_hw_done() that flushes and waits for flip done before calling drm_atomic_helper_commit_hw_done(). The fix was prompted by a patch from Carsten Haitzler where he was trying to solve the same issue but in a different way that I think can lead to wrong event signaling to userspace. Reported-by: Carsten Haitzler <carsten.haitzler@arm.com> Tested-by: Carsten Haitzler <carsten.haitzler@arm.com> Reviewed-by: Carsten Haitzler <carsten.haitzler@arm.com> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com> Link: https://patchwork.freedesktop.org/patch/msgid/20220722122139.288486-1-liviu.dudau@arm.com Signed-off-by: Sasha Levin <sashal@kernel.org> --- .../gpu/drm/arm/display/komeda/komeda_crtc.c | 4 ++-- .../gpu/drm/arm/display/komeda/komeda_kms.c | 21 ++++++++++++++++++- .../gpu/drm/arm/display/komeda/komeda_kms.h | 2 ++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c index 59172acb9738..292f533d8cf0 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c @@ -235,7 +235,7 @@ void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, crtc->state->event = NULL; drm_crtc_send_vblank_event(crtc, event); } else { - DRM_WARN("CRTC[%d]: FLIP happen but no pending commit.\n", + DRM_WARN("CRTC[%d]: FLIP happened but no pending commit.\n", drm_crtc_index(&kcrtc->base)); } spin_unlock_irqrestore(&crtc->dev->event_lock, flags); @@ -286,7 +286,7 @@ komeda_crtc_atomic_enable(struct drm_crtc *crtc, komeda_crtc_do_flush(crtc, old); } -static void +void komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc, struct completion *input_flip_done) { diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index 93b7f09b96ca..327051bba5b6 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -69,6 +69,25 @@ static const struct drm_driver komeda_kms_driver = { .minor = 1, }; +static void komeda_kms_atomic_commit_hw_done(struct drm_atomic_state *state) +{ + struct drm_device *dev = state->dev; + struct komeda_kms_dev *kms = to_kdev(dev); + int i; + + for (i = 0; i < kms->n_crtcs; i++) { + struct komeda_crtc *kcrtc = &kms->crtcs[i]; + + if (kcrtc->base.state->active) { + struct completion *flip_done = NULL; + if (kcrtc->base.state->event) + flip_done = kcrtc->base.state->event->base.completion; + komeda_crtc_flush_and_wait_for_flip_done(kcrtc, flip_done); + } + } + drm_atomic_helper_commit_hw_done(state); +} + static void komeda_kms_commit_tail(struct drm_atomic_state *old_state) { struct drm_device *dev = old_state->dev; @@ -81,7 +100,7 @@ static void komeda_kms_commit_tail(struct drm_atomic_state *old_state) drm_atomic_helper_commit_modeset_enables(dev, old_state); - drm_atomic_helper_commit_hw_done(old_state); + komeda_kms_atomic_commit_hw_done(old_state); drm_atomic_helper_wait_for_flip_done(dev, old_state); diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h index 456f3c435719..bf6e8fba5061 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.h +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.h @@ -182,6 +182,8 @@ void komeda_kms_cleanup_private_objs(struct komeda_kms_dev *kms); void komeda_crtc_handle_event(struct komeda_crtc *kcrtc, struct komeda_events *evts); +void komeda_crtc_flush_and_wait_for_flip_done(struct komeda_crtc *kcrtc, + struct completion *input_flip_done); struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev); void komeda_kms_detach(struct komeda_kms_dev *kms); -- 2.35.1
next prev parent reply other threads:[~2022-10-09 23:54 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-10-09 23:54 [PATCH AUTOSEL 5.15 01/25] drm/nouveau/nouveau_bo: fix potential memory leak in nouveau_bo_alloc() Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [Nouveau] " Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 02/25] drm: Use size_t type for len variable in drm_copy_field() Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 03/25] drm: Prevent drm_copy_field() to attempt copying a NULL pointer Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin [this message] 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 04/25] drm/komeda: Fix handling of atomic commits in the atomic_commit_tail hook Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 05/25] gpu: lontium-lt9611: Fix NULL pointer dereference in lt9611_connector_init() Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 06/25] drm/amd/display: fix overflow on MIN_I64 definition Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 07/25] ALSA: usb-audio: Add quirk to enable Avid Mbox 3 support Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 08/25] udmabuf: Set ubuf->sg = NULL if the creation of sg table fails Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 09/25] drm: bridge: dw_hdmi: only trigger hotplug event on link change Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 10/25] drm: hide unregistered connectors from GETCONNECTOR IOCTL Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 11/25] ALSA: usb-audio: Register card at the last interface Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 12/25] drm/vc4: vec: Fix timings for VEC modes Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 13/25] ACPI: video: Change disable_backlight_sysfs_if quirks to acpi_backlight=native Sasha Levin 2022-10-10 7:36 ` Hans de Goede 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 14/25] drm: panel-orientation-quirks: Add quirk for Anbernic Win600 Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 15/25] platform/chrome: cros_ec: Notify the PM of wake events during resume Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 16/25] platform/x86: msi-laptop: Change DMI match / alias strings to fix module autoloading Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 17/25] ASoC: SOF: pci: Change DMI match info to support all Chrome platforms Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 18/25] drm/amdgpu: fix initial connector audio value Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 19/25] drm/meson: reorder driver deinit sequence to fix use-after-free bug Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 20/25] drm/meson: explicitly remove aggregate driver at module unload time Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 21/25] drm/exynos: Fix return type for mixer_mode_valid and hdmi_mode_valid Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 22/25] mmc: sdhci-msm: add compatible string check for sdm670 Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 23/25] drm/dp: Don't rewrite link config when setting phy test pattern Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 24/25] drm/amd/display: Remove interface for periodic interrupt 1 Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` [PATCH AUTOSEL 5.15 25/25] drm/amd/display: fix array-bounds error in dc_stream_remove_writeback() Sasha Levin 2022-10-09 23:54 ` Sasha Levin 2022-10-09 23:54 ` Sasha Levin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20221009235426.1231313-4-sashal@kernel.org \ --to=sashal@kernel.org \ --cc=carsten.haitzler@arm.com \ --cc=christian.koenig@amd.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=james.qian.wang@arm.com \ --cc=linaro-mm-sig@lists.linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=liviu.dudau@arm.com \ --cc=mihail.atanassov@arm.com \ --cc=stable@vger.kernel.org \ --cc=sumit.semwal@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.