On 29/08/2024 15:53, Geert Uytterhoeven wrote:
On Thu, May 30, 2024 at 10:33 AM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
On Thu, 30 May 2024 at 11:16, Jocelyn Falempe <jfalempe@xxxxxxxxxx> wrote:
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:
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.
I think there should be a default implementation which fits 80% of the
cases (single fixed PRIMARY plane per output) but if the driver needs
it, it should be able to override the behaviour.
Did anything like this materialize, or is this patch (and its rcar-du
counterpart [1]) good to apply as-is?
No I didn't find a better option yet, and I think it is good as-is.
If duplicating the helper funcs is really a problem, I think the less
intrusive option, would be to add a "bool panic_allow_overlay_plane" to
struct drm_mode_config, and use that in drm_panic_register(), to
register only primary planes if it's not set ?
So drivers that want to draw the panic on overlay plane can opt-in.
Best regards,
--
Jocelyn
Thank you!
[1] https://lore.kernel.org/r/b633568d2e3f405b21debdd60854fe39780254d6.1716816897.git.geert+renesas@xxxxxxxxx/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
