Re: [PATCH] drm/mcde: Fix stability issue

2020-07-26 Thread Stephan Gerhold
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

2020-07-26 Thread Sam Ravnborg
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

2020-07-21 Thread Sasha Levin
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 

[PATCH] drm/mcde: Fix stability issue

2020-07-18 Thread Linus Walleij
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