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] [day] [month] [year] [list]

Message-ID: <7a12280a-a7a8-4b5a-b085-90b7fe1728fe@linaro.org>
Date: Wed, 2 Oct 2024 13:19:20 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Maxime Ripard <mripard@...nel.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

Hi,

On 02/10/2024 13:02, Maxime Ripard wrote:
> + 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?

Yes it does, "compressed" is in he way pixels are ordered in memory, meaning
it will be faster to scanout when the image is simple, but with a fully random
image the buffer will be larger.

But per my comment, AFBC is really only used on Android producst since it requires
DRM_FORMAT_XBGR8888, so the best is to bail out when AFBC is enabled.

Neil

> 
> Maxime