linux-kernel - Re: [RFC PATCH 1/1] drm/meson: Support drm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241002-vengeful-vivacious-salamander-cdc9f7@houat>
Date: Wed, 2 Oct 2024 13:02:28 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Neil Armstrong <neil.armstrong@...aro.org>, 
	Jocelyn Falempe <jfalempe@...hat.com>
Cc: Yao Zi <ziyao@...root.org>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Thomas Zimmermann <tzimmermann@...e.de>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Kevin Hilman <khilman@...libre.com>, Jerome Brunet <jbrunet@...libre.com>, 
	Martin Blumenstingl <martin.blumenstingl@...glemail.com>, dri-devel@...ts.freedesktop.org, 
	linux-amlogic@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/1] drm/meson: Support drm_panic

+ Jocelyn

On Wed, Oct 02, 2024 at 09:59:57AM GMT, Neil Armstrong wrote:
> > diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> > index b43ac61201f3..b2def784c00d 100644
> > --- a/drivers/gpu/drm/meson/meson_plane.c
> > +++ b/drivers/gpu/drm/meson/meson_plane.c
> > @@ -20,6 +20,8 @@
> >   #include <drm/drm_framebuffer.h>
> >   #include <drm/drm_gem_atomic_helper.h>
> >   #include <drm/drm_gem_dma_helper.h>
> > +#include <drm/drm_gem_framebuffer_helper.h>
> > +#include <drm/drm_panic.h>
> >   #include "meson_plane.h"
> >   #include "meson_registers.h"
> > @@ -419,10 +421,49 @@ static void meson_plane_atomic_disable(struct drm_plane *plane,
> >   	priv->viu.osd1_enabled = false;
> >   }
> > +static int meson_plane_get_scanout_buffer(struct drm_plane *plane,
> > +					  struct drm_scanout_buffer *sb)
> > +{
> > +	struct meson_plane *meson_plane = to_meson_plane(plane);
> > +	struct meson_drm *priv = meson_plane->priv;
> > +	struct drm_framebuffer *fb;
> > +
> > +	if (!meson_plane->enabled)
> > +		return -ENODEV;
> > +
> > +	if (priv->viu.osd1_afbcd) {
> > +		if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM)) {
> 
> This should be meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)
> 
> You should call:
> 
> 			if (priv->afbcd.ops) {
> 				priv->afbcd.ops->reset(priv);
> 				priv->afbcd.ops->disable(priv);
> 			}

I'm not sure it's a good idea. This code is run in the panic path, and
it comes with strict requirements that these functions don't have.

It'll be incredibly easy to add a sleeping call or a lock in there.

On a more fundamental level, I'm not sure we should be disableing AFBC
at all. Do we even have the guarantee that the buffer is large enough to
hold XRGB8888 pixels?

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)