[PATCH] drm: rcar-du: Fix crash when enabling a non-visible plane

[PATCH] drm: rcar-du: Fix crash when enabling a non-visible plane

[Laurent Pinchart]

The DU driver handles non-visible planes (fully clipped by the display's
boundaries) by considering them as disabled. It thus disables the plane
at the hardware level when the plane if moved off-screen. However, if
the plane was previously disabled and is non-visible when it gets
enabled, the attempt to disable it crashes, ad the plane wasn't
previously enabled. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index f1a81c9b184d..ff233a7b398d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -279,7 +279,7 @@ static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane,
 
 	if (plane->state->visible)
 		rcar_du_vsp_plane_setup(rplane);
-	else
+	else if (old_state->crtc)
 		vsp1_du_atomic_update(rplane->vsp->vsp, crtc->vsp_pipe,
 				      rplane->index, NULL);
 }
-- 
Regards,

Laurent Pinchart

Re: [PATCH] drm: rcar-du: Fix crash when enabling a non-visible plane

[Geert Uytterhoeven]

On Fri, Aug 7, 2020 at 11:23 PM Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The DU driver handles non-visible planes (fully clipped by the display's
> boundaries) by considering them as disabled. It thus disables the plane
> at the hardware level when the plane if moved off-screen. However, if

is moved

> the plane was previously disabled and is non-visible when it gets
> enabled, the attempt to disable it crashes, ad the plane wasn't

as

> previously enabled. Fix it.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] drm: rcar-du: Fix crash when enabling a non-visible plane

[Kieran Bingham]

Hi Laurent,

On 07/08/2020 22:22, Laurent Pinchart wrote:
> The DU driver handles non-visible planes (fully clipped by the display's
> boundaries) by considering them as disabled. It thus disables the plane
> at the hardware level when the plane if moved off-screen. However, if
> the plane was previously disabled and is non-visible when it gets
> enabled, the attempt to disable it crashes, ad the plane wasn't

"as the plane"

And now I see Geert beat me to that one anyway.

> previously enabled. Fix it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> index f1a81c9b184d..ff233a7b398d 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> @@ -279,7 +279,7 @@ static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane,
>  
>  	if (plane->state->visible)
>  		rcar_du_vsp_plane_setup(rplane);
> -	else
> +	else if (old_state->crtc)

Aha, I wondered why we weren't just referencing crtc here, but I see
it's a container_of so not necessarily 'null' when invalid. :-S


>  		vsp1_du_atomic_update(rplane->vsp->vsp, crtc->vsp_pipe,
>  				      rplane->index, NULL);

I presume that the crash was that the crtc pointer here was invalid in
this instance.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


>  }
> 

-- 
Regards
--
Kieran