[RFC PATCH 1/1] drm/meson: Support drm_panic
Jocelyn Falempe
jfalempe at redhat.com
Thu Oct 3 07:52:33 UTC 2024
On 02/10/2024 09:59, Neil Armstrong wrote:
> Hi !
>
> On 01/10/2024 23:04, Yao Zi wrote:
>> This patch implements drm_plane_helper_funcs.get_scanout_buffer for
>> primary plane, enabling meson-drm to work with drm_panic.
>>
>> This implementation tries to use current framebuffer as scanout buffer.
>> In case of AFBC enabled, we disable the decoder path and adjust OSD1
>> parameters in get_scanout_buffer to make the buffer linear.
>>
>> Tested on TTY and Wayland session (Sway).
>
> Thanks for enabling this!
>
>>
>> Signed-off-by: Yao Zi <ziyao at disroot.org>
>> ---
>> drivers/gpu/drm/meson/meson_plane.c | 47 +++++++++++++++++++++++++++--
>> 1 file changed, 44 insertions(+), 3 deletions(-)
>>
>> 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);
> }
>
>> + writel_relaxed(0, priv->io_base +
>> + _REG(VIU_OSD1_BLK1_CFG_W4));
>> + writel_relaxed(0, priv->io_base +
>> + _REG(VIU_OSD1_BLK2_CFG_W4));
>> + writel_bits_relaxed(OSD_ENDIANNESS_LE, OSD_ENDIANNESS_LE,
>> + priv->io_base +
>> + _REG(VIU_OSD1_BLK0_CFG_W0));
>
> This won't work, drop it, the canvas isn't correctly configured, you
> should instead call:
> meson_canvas_config(priv->canvas, priv->canvas_id_osd1,
> priv->viu.osd1_addr,
> priv->viu.osd1_stride,
> priv->viu.osd1_height,
> MESON_CANVAS_WRAP_NONE,
> MESON_CANVAS_BLKMODE_LINEAR, 0);
The problem is meson_canvas_config() takes a spin_lock, which can block
forever in a panic handler.
So you can rewrite a version without lock.
Considering the addr, stride, and height don't change in a panic, it can
probably be simplified for this case.
Also this will be the last image to be drawn, and the last driver code
to be executed before a reboot, so it's ok if it's a bit dirty, and if
the software state is not up-to-date with the hardware state.
>
>> + meson_viu_g12a_disable_osd1_afbc(priv);
>> + } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>
> And here meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM)
>
>> + writel_bits_relaxed(OSD_DPATH_MALI_AFBCD, 0,
>> + priv->io_base +
>> + _REG(VIU_OSD1_CTRL_STAT2));
>
> Ok, you should also call meson_canvas_config()
>
> You should call:
>
> if (priv->afbcd.ops) {
> priv->afbcd.ops->reset(priv);
> priv->afbcd.ops->disable(priv);
> }
>
>> + meson_viu_gxm_disable_osd1_afbc(priv);
>> + }
>> + }
>
> I thing the code should look like:
>
> if (priv->viu.osd1_afbcd) {
> meson_canvas_config(priv->canvas, priv->canvas_id_osd1,
> priv->viu.osd1_addr,
> priv->viu.osd1_stride,
> priv->viu.osd1_height,
> MESON_CANVAS_WRAP_NONE,
> MESON_CANVAS_BLKMODE_LINEAR, 0);
>
> if (priv->afbcd.ops) {
> priv->afbcd.ops->reset(priv);
> priv->afbcd.ops->disable(priv);
> }
>
> if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
> writel_bits_relaxed(OSD_ENDIANNESS_LE, OSD_ENDIANNESS_LE,
> priv->io_base +
> _REG(VIU_OSD1_BLK0_CFG_W0));
> meson_viu_g12a_disable_osd1_afbc(priv);
> } else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM)) {
> writel_bits_relaxed(OSD_DPATH_MALI_AFBCD, 0,
> priv->io_base +
> _REG(VIU_OSD1_CTRL_STAT2));
> meson_viu_gxm_disable_osd1_afbc(priv);
> }
> }
>
> AFBC is quite hard to test since it requires DRM_FORMAT_XBGR8888, but
> I think sway should perhaps support it, Mesa should also support AFBC.
>
> At some point I made some memory dumps of AFBC buffers, perhaps they could
> be useful here.
>
> Another way would be to simply ignore the AFBC case, and bail out since
> it would be a very rare case.
panic handler is a best effort mode, so if it's too complex to support
AFBC, and it's not very widespread, then it's ok to bail out in this case.
>
>> +
>> + fb = plane->state->fb;
>> + sb->format = fb->format;
>> + sb->width = fb->width;
>> + sb->height = fb->height;
>> + sb->pitch[0] = fb->pitches[0];
>> + drm_gem_fb_vmap(fb, sb->map, NULL);
>> +
>> + return 0;
>> +}
>> +
>> static const struct drm_plane_helper_funcs meson_plane_helper_funcs = {
>> - .atomic_check = meson_plane_atomic_check,
>> - .atomic_disable = meson_plane_atomic_disable,
>> - .atomic_update = meson_plane_atomic_update,
>> + .atomic_check = meson_plane_atomic_check,
>> + .atomic_disable = meson_plane_atomic_disable,
>> + .atomic_update = meson_plane_atomic_update,
>> + .get_scanout_buffer = meson_plane_get_scanout_buffer,
>> };
>> static bool meson_plane_format_mod_supported(struct drm_plane *plane,
>
> Thanks,
> Neil
>
Best regards,
--
Jocelyn
More information about the dri-devel
mailing list