On 29/05/2024 15:33, Laurent Pinchart wrote:
On Wed, May 29, 2024 at 04:28:44PM +0300, Dmitry Baryshkov wrote:
On Wed, May 29, 2024 at 12:55:06PM +0300, Laurent Pinchart wrote:
On Wed, May 29, 2024 at 12:25:56PM +0300, Dmitry Baryshkov wrote:
On Wed, May 29, 2024 at 12:10:18PM +0300, Laurent Pinchart wrote:
Hi Dmitry,
On Wed, May 29, 2024 at 11:27:02AM +0300, Dmitry Baryshkov wrote:
On Wed, May 29, 2024 at 04:03:20AM +0300, Laurent Pinchart wrote:
On Mon, May 27, 2024 at 03:34:48PM +0200, Geert Uytterhoeven wrote:
Add support for the drm_panic module, which displays a message on
the screen when a kernel panic occurs.
Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
---
Tested on Armadillo-800-EVA.
---
drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
index 07ad17d24294d5e6..9d166ab2af8bd231 100644
--- a/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
+++ b/drivers/gpu/drm/renesas/shmobile/shmob_drm_plane.c
@@ -273,6 +273,13 @@ static const struct drm_plane_helper_funcs shmob_drm_plane_helper_funcs = {
.atomic_disable = shmob_drm_plane_atomic_disable,
};
+static const struct drm_plane_helper_funcs shmob_drm_primary_plane_helper_funcs = {
+ .atomic_check = shmob_drm_plane_atomic_check,
+ .atomic_update = shmob_drm_plane_atomic_update,
+ .atomic_disable = shmob_drm_plane_atomic_disable,
+ .get_scanout_buffer = drm_fb_dma_get_scanout_buffer,
+};
+
static const struct drm_plane_funcs shmob_drm_plane_funcs = {
.update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
@@ -310,7 +317,12 @@ struct drm_plane *shmob_drm_plane_create(struct shmob_drm_device *sdev,
splane->index = index;
- drm_plane_helper_add(&splane->base, &shmob_drm_plane_helper_funcs);
+ if (type == DRM_PLANE_TYPE_PRIMARY)
+ drm_plane_helper_add(&splane->base,
+ &shmob_drm_primary_plane_helper_funcs);
+ else
+ drm_plane_helper_add(&splane->base,
+ &shmob_drm_plane_helper_funcs);
It's not very nice to have to provide different operations for the
primary and overlay planes. The documentation of
drm_fb_dma_get_scanout_buffer() states
* @plane: DRM primary plane
If the intent is that only primary planes will be used to display the
panic message, shouldn't drm_panic_register() skip overlay planes ? It
would simplify drivers.
What about the drivers where all the planes are actually universal?
In such a case the planes registered as primary can easily get replaced
by 'overlay' planes.
Good point.
Another option, if we wanted to avoid duplicating the drm_plane_funcs,
would be to add a field to drm_plane to indicate whether the plane is
suitable for drm_panic.
... or maybe let the driver decide. For the fully-universal-plane
devices we probably want to select the planes which cover the largest
part of the CRTC.
Are there devices where certain planes can only cover a subset of the
CRTC (apart from planes meant for cursors) ?
On contemporary MSM devices any plane can cover any part of the screen,
including not having a plane that covers the full screen at all.
Ah, you meant picking the plane that is currently covering most of the
screen. I thought you were talking about devices where some planes were
restricted by the hardware to a subset of the CRTC.
I agree it would make sense to take both plane position and z-pos, as
well as visibility and other related parameters, to pick the plane that
is the most visible. Ideally this should be handled in drm_panic, not
duplicated in drivers.
I'm not sure that drm_panic can figure out reliably on which plane it
needs to draw. I think the driver has more information to take the right
decision.
Also if you prefer, you can add the get_scanout_buffer() callback for
all planes (to use the same helper fops), and then filter out in the
callback for planes that are not suitable. I just find it cleaner to not
register planes that the driver knows they will never be suitable (like
cursor planes).
static void shmob_atomic_helper_get_scanout_buffer(struct drm_plane
*plane, struct drm_scanout_buffer *sb))
{
if (plane->type == DRM_PLANE_TYPE_PRIMARY)
return drm_fb_dma_get_scanout_buffer(plane, sb);
return -EOPNOTSUPP;
}
.get_scanout_buffer = shmob_atomic_helper_get_scanout_buffer,
--
Jocelyn
I think that what would matter the most in the end is selecting the
plane that is on top of the stack, and that doesn't seem to be addressed
by the drm_panic infrastructure. This is getting out of scope for this
patch though :-)
I don't think this patch should be blocked just for this reason, but I'm
a bit bothered by duplicating the ops structure to indicate drm_panic
support.
return &splane->base;
}
