Re: [PATCH] drm/mcde: Fix stability issue
On Sun, Jul 19, 2020 at 01:33:22AM +0200, Linus Walleij wrote: > Whenener a display update was sent, apart from updating > the memory base address we called mcde_display_send_one_frame() > which also sent a command to the display requesting the TE IRQ > and enabling the FIFO. > > When continous updates are running this is wrong: we need > to only send this to start the flow to the display on > the very first update. This lead to the display pipeline > locking up and crashing. > > Check if the flow is already running and in that case > do not call mcde_display_send_one_frame(). > > This fixes crashes on the Samsung GT-S7710 (Skomer). > > Cc: Stephan Gerhold > Cc: sta...@vger.kernel.org > Signed-off-by: Linus Walleij > --- > drivers/gpu/drm/mcde/mcde_display.c | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/mcde/mcde_display.c > b/drivers/gpu/drm/mcde/mcde_display.c > index 212aee60cf61..1d8ea8830a17 100644 > --- a/drivers/gpu/drm/mcde/mcde_display.c > +++ b/drivers/gpu/drm/mcde/mcde_display.c > @@ -1086,9 +1086,14 @@ static void mcde_display_update(struct > drm_simple_display_pipe *pipe, >*/ > if (fb) { > mcde_set_extsrc(mcde, drm_fb_cma_get_gem_addr(fb, pstate, 0)); > - if (!mcde->video_mode) > - /* Send a single frame using software sync */ > - mcde_display_send_one_frame(mcde); > + if (!mcde->video_mode) { > + /* > + * Send a single frame using software sync if the flow > + * is not active yet. > + */ > + if (mcde->flow_active == 0) > + mcde_display_send_one_frame(mcde); > + } I think this makes sense as a fix for the issue you described, so FWIW: Acked-by: Stephan Gerhold While looking at this I had a few thoughts for potential future patches: - Clearly mcde_display_send_one_frame() does not only send a single frame only in some cases (when te_sync = true), so maybe it should be named differently? - I was a bit confused because with this change we also call mcde_dsi_te_request() only once. Looking at the vendor driver the nova_dsilink_te_request() function that is very similar is only called within mcde_add_bta_te_oneshot_listener(), which is only called for MCDE_SYNCSRC_BTA. However, the rest of the MCDE code looks more similar to MCDE_SYNCSRC_TE0, which does not call that function in the vendor driver. I wonder if mcde_dsi_te_request() is needed at all? Thanks, Stephan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mcde: Fix stability issue
Hi Linus. On Sun, Jul 19, 2020 at 01:33:22AM +0200, Linus Walleij wrote: > Whenener a display update was sent, apart from updating s/Whenener/Whenever > the memory base address we called mcde_display_send_one_frame() ^ insert comma? > which also sent a command to the display requesting the TE IRQ > and enabling the FIFO. > > When continous updates are running this is wrong: we need > to only send this to start the flow to the display on > the very first update. This lead to the display pipeline > locking up and crashing. > > Check if the flow is already running and in that case > do not call mcde_display_send_one_frame(). > > This fixes crashes on the Samsung GT-S7710 (Skomer). > > Cc: Stephan Gerhold > Cc: sta...@vger.kernel.org > Signed-off-by: Linus Walleij Patch looks fine. Acked-by: Sam Ravnborg > --- > drivers/gpu/drm/mcde/mcde_display.c | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/mcde/mcde_display.c > b/drivers/gpu/drm/mcde/mcde_display.c > index 212aee60cf61..1d8ea8830a17 100644 > --- a/drivers/gpu/drm/mcde/mcde_display.c > +++ b/drivers/gpu/drm/mcde/mcde_display.c > @@ -1086,9 +1086,14 @@ static void mcde_display_update(struct > drm_simple_display_pipe *pipe, >*/ > if (fb) { > mcde_set_extsrc(mcde, drm_fb_cma_get_gem_addr(fb, pstate, 0)); > - if (!mcde->video_mode) > - /* Send a single frame using software sync */ > - mcde_display_send_one_frame(mcde); > + if (!mcde->video_mode) { > + /* > + * Send a single frame using software sync if the flow > + * is not active yet. > + */ > + if (mcde->flow_active == 0) > + mcde_display_send_one_frame(mcde); > + } > dev_info_once(mcde->dev, "sent first display update\n"); > } else { > /* > -- > 2.26.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/mcde: Fix stability issue
Hi [This is an automated email] This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all The bot has tested the following trees: v5.7.9, v5.4.52, v4.19.133, v4.14.188, v4.9.230, v4.4.230. v5.4.52: Failed to apply! Possible dependencies: 768859c239922 ("drm/mcde: Provide vblank handling unconditionally") d920e8da3d837 ("drm/mcde: Fix frame sync setup for video mode panels") v4.19.133: Failed to apply! Possible dependencies: 01648890f336a ("staging: vboxvideo: Embed drm_device into driver structure") 0424d7ba4574b ("staging: vboxvideo: Init fb_info.fix.smem once from fbdev_create") 0fdda2ce74e5f ("staging: vboxvideo: Move pin / unpin of fb out of vbox_crtc_set_base_and_mode") 131abc56e1bac ("drm/vboxvideo: Move the vboxvideo driver out of staging") 3498ea8b7e3c8 ("staging: vboxvideo: Fold vbox_drm_resume() into vbox_pm_resume()") 35f3288c453e2 ("staging: vboxvideo: Atomic phase 1: convert cursor to universal plane") 438340aa20975 ("staging: vboxvideo: Atomic phase 3: Switch last bits over to atomic") 4f2a8f5898ecd ("drm: Add ASPEED GFX driver") 5cf5332d529bf ("staging: vboxvideo: Restore page-flip support") 5fc537bfd0003 ("drm/mcde: Add new driver for ST-Ericsson MCDE") 685bb884e0a42 ("staging: vboxvideo: Drop duplicate vbox_err.h file") 79815ee23890c ("staging: vboxvideo: Move setup of modesetting from driver_load to mode_init") 96bae04347b24 ("staging/vboxvideo: prepare for drmP.h removal from drm_modeset_helper.h") a1d2a6339961e ("drm/lima: driver for ARM Mali4xx GPUs") a5aca20574693 ("staging: vboxvideo: Fix modeset / page_flip error handling") acc962c514007 ("staging: vboxvideo: Change licence headers over to SPDX") cb5eaf187d1d9 ("staging: vboxvideo: Expose creation of universal primary plane") cc0ec5eb721f1 ("staging: vboxvideo: Atomic phase 1: Use drm_plane_helpers for primary plane") cd76c287a52fe ("staging: vboxvideo: Cleanup the comments") ce8ec32cbd420 ("staging: vboxvideo: Remove vboxfb_create_object() wrapper") cfc1fc63be447 ("staging: vboxvideo: Move bo_[un]resere calls into vbox_bo_[un]pin") d46709094deb6 ("staging: vboxvideo: Fold driver_load/unload into probe/remove functions") f4d6d90f83147 ("staging: vboxvideo: Add fl_flag argument to vbox_fb_pin() helper") v4.14.188: Failed to apply! Possible dependencies: 01648890f336a ("staging: vboxvideo: Embed drm_device into driver structure") 0424d7ba4574b ("staging: vboxvideo: Init fb_info.fix.smem once from fbdev_create") 0fdda2ce74e5f ("staging: vboxvideo: Move pin / unpin of fb out of vbox_crtc_set_base_and_mode") 131abc56e1bac ("drm/vboxvideo: Move the vboxvideo driver out of staging") 179c02fe90a41 ("drm/tve200: Add new driver for TVE200") 1daddbc8dec56 ("staging: vboxvideo: Update driver to use drm_dev_register.") 1ebafd1561a05 ("staging: vboxvideo: Fix IRQs no longer working") 2408898e3b6c9 ("staging: vboxvideo: Add page-flip support") 3498ea8b7e3c8 ("staging: vboxvideo: Fold vbox_drm_resume() into vbox_pm_resume()") 35f3288c453e2 ("staging: vboxvideo: Atomic phase 1: convert cursor to universal plane") 438340aa20975 ("staging: vboxvideo: Atomic phase 3: Switch last bits over to atomic") 4f2a8f5898ecd ("drm: Add ASPEED GFX driver") 5b8ea816e8e05 ("drm/tinydrm: add driver for ST7735R panels") 5cf5332d529bf ("staging: vboxvideo: Restore page-flip support") 5fc537bfd0003 ("drm/mcde: Add new driver for ST-Ericsson MCDE") 65aac17423284 ("staging: vboxvideo: Change address of scanout buffer on page-flip") 685bb884e0a42 ("staging: vboxvideo: Drop duplicate vbox_err.h file") 6d544fd6f4e15 ("drm/doc: Put all driver docs into a separate chapter") 79815ee23890c ("staging: vboxvideo: Move setup of modesetting from driver_load to mode_init") 96bae04347b24 ("staging/vboxvideo: prepare for drmP.h removal from drm_modeset_helper.h") a1d2a6339961e ("drm/lima: driver for ARM Mali4xx GPUs") a5aca20574693 ("staging: vboxvideo: Fix modeset / page_flip error handling") acc962c514007 ("staging: vboxvideo: Change licence headers over to SPDX") ba67f54d911c3 ("staging: vboxvideo: Pass a new framebuffer to vbox_crtc_do_set_base") c575b7eeb89f9 ("drm/xen-front: Add support for Xen PV display frontend") cb5eaf187d1d9 ("staging: vboxvideo: Expose creation of universal primary plane") cc0ec5eb721f1 ("staging: vboxvideo: Atomic phase 1: Use drm_plane_helpers for primary plane") cd76c287a52fe ("staging: vboxvideo: Cleanup the comments") ce8ec32cbd420 ("staging: vboxvideo: Remove vboxfb_create_object() wrapper") cfc1fc63be447 ("staging: vboxvideo: Move bo_[un]resere calls into vbox_bo_[un]pin") d46709094deb6 ("staging: vboxvideo: Fold driver_load/unload into probe/remove functions") f4d6d90f83147 ("staging: vboxvideo: Add fl_flag argumen
[PATCH] drm/mcde: Fix stability issue
Whenener a display update was sent, apart from updating the memory base address we called mcde_display_send_one_frame() which also sent a command to the display requesting the TE IRQ and enabling the FIFO. When continous updates are running this is wrong: we need to only send this to start the flow to the display on the very first update. This lead to the display pipeline locking up and crashing. Check if the flow is already running and in that case do not call mcde_display_send_one_frame(). This fixes crashes on the Samsung GT-S7710 (Skomer). Cc: Stephan Gerhold Cc: sta...@vger.kernel.org Signed-off-by: Linus Walleij --- drivers/gpu/drm/mcde/mcde_display.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c index 212aee60cf61..1d8ea8830a17 100644 --- a/drivers/gpu/drm/mcde/mcde_display.c +++ b/drivers/gpu/drm/mcde/mcde_display.c @@ -1086,9 +1086,14 @@ static void mcde_display_update(struct drm_simple_display_pipe *pipe, */ if (fb) { mcde_set_extsrc(mcde, drm_fb_cma_get_gem_addr(fb, pstate, 0)); - if (!mcde->video_mode) - /* Send a single frame using software sync */ - mcde_display_send_one_frame(mcde); + if (!mcde->video_mode) { + /* +* Send a single frame using software sync if the flow +* is not active yet. +*/ + if (mcde->flow_active == 0) + mcde_display_send_one_frame(mcde); + } dev_info_once(mcde->dev, "sent first display update\n"); } else { /* -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel