| Message ID | 20211018142842.2511200-9-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/omap: Add virtual-planes support | expand |
On 18/10/2021 17:28, Neil Armstrong wrote: > From: Benoit Parrot <bparrot@ti.com> > > Now that we added specific item to our subclassed drm_plane_state > we can add omap_plane_atomic_print_state() helper to dump out our own > driver specific plane state. > > Signed-off-by: Benoit Parrot <bparrot@ti.com> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > --- > drivers/gpu/drm/omapdrm/omap_plane.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c > index ce5ed45401fb..5001c8354e4f 100644 > --- a/drivers/gpu/drm/omapdrm/omap_plane.c > +++ b/drivers/gpu/drm/omapdrm/omap_plane.c > @@ -348,6 +348,21 @@ omap_plane_atomic_duplicate_state(struct drm_plane *plane) > return &state->base; > } > > +static void omap_plane_atomic_print_state(struct drm_printer *p, > + const struct drm_plane_state *state) > +{ > + struct omap_plane_state *omap_state = to_omap_plane_state(state); > + > + drm_printf(p, "\toverlay=%s\n", omap_state->overlay ? > + omap_state->overlay->name : "(null)"); > + if (omap_state->overlay) { > + drm_printf(p, "\t\tidx=%d\n", omap_state->overlay->idx); > + drm_printf(p, "\t\toverlay_id=%d\n", > + omap_state->overlay->id); > + drm_printf(p, "\t\tcaps=0x%x\n", omap_state->overlay->caps); > + } > +} This prints: overlay=gfx idx=0 overlay_id=0 caps=0x3e I'm not sure if some of these details are needed. The name ("gfx") and overlay_id refer to the same thing, and while idx is in theory a different value, in practice it's always the same as overlay_id. And even if it was a different number, I think idx is kind of irrelevant, isn't it? caps can be figured out from the name of the overlay, but perhaps it doesn't hurt to print them here. Then again, if none of the other debug prints show the cap values (e.g. "requires cap 0x4), maybe printing the caps value is not really useful here. Maybe this could be just a single line, say: overlay=gfx or if caps is useful: overlay=gfx (caps=0x3e) What do you think? Tomi
On 27/10/2021 14:23, Tomi Valkeinen wrote: > On 18/10/2021 17:28, Neil Armstrong wrote: >> From: Benoit Parrot <bparrot@ti.com> >> >> Now that we added specific item to our subclassed drm_plane_state >> we can add omap_plane_atomic_print_state() helper to dump out our own >> driver specific plane state. >> >> Signed-off-by: Benoit Parrot <bparrot@ti.com> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> >> --- >> drivers/gpu/drm/omapdrm/omap_plane.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c >> index ce5ed45401fb..5001c8354e4f 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_plane.c >> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c >> @@ -348,6 +348,21 @@ omap_plane_atomic_duplicate_state(struct drm_plane *plane) >> return &state->base; >> } >> +static void omap_plane_atomic_print_state(struct drm_printer *p, >> + const struct drm_plane_state *state) >> +{ >> + struct omap_plane_state *omap_state = to_omap_plane_state(state); >> + >> + drm_printf(p, "\toverlay=%s\n", omap_state->overlay ? >> + omap_state->overlay->name : "(null)"); >> + if (omap_state->overlay) { >> + drm_printf(p, "\t\tidx=%d\n", omap_state->overlay->idx); >> + drm_printf(p, "\t\toverlay_id=%d\n", >> + omap_state->overlay->id); >> + drm_printf(p, "\t\tcaps=0x%x\n", omap_state->overlay->caps); >> + } >> +} > > This prints: > > overlay=gfx > idx=0 > overlay_id=0 > caps=0x3e > > I'm not sure if some of these details are needed. The name ("gfx") and overlay_id refer to the same thing, and while idx is in theory a different value, in practice it's always the same as overlay_id. And even if it was a different number, I think idx is kind of irrelevant, isn't it? > > caps can be figured out from the name of the overlay, but perhaps it doesn't hurt to print them here. Then again, if none of the other debug prints show the cap values (e.g. "requires cap 0x4), maybe printing the caps value is not really useful here. > > Maybe this could be just a single line, say: > > overlay=gfx > > or if caps is useful: > > overlay=gfx (caps=0x3e) > > What do you think? I'm ok with that. Thanks, Neil > > Tomi
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index ce5ed45401fb..5001c8354e4f 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -348,6 +348,21 @@ omap_plane_atomic_duplicate_state(struct drm_plane *plane) return &state->base; } +static void omap_plane_atomic_print_state(struct drm_printer *p, + const struct drm_plane_state *state) +{ + struct omap_plane_state *omap_state = to_omap_plane_state(state); + + drm_printf(p, "\toverlay=%s\n", omap_state->overlay ? + omap_state->overlay->name : "(null)"); + if (omap_state->overlay) { + drm_printf(p, "\t\tidx=%d\n", omap_state->overlay->idx); + drm_printf(p, "\t\toverlay_id=%d\n", + omap_state->overlay->id); + drm_printf(p, "\t\tcaps=0x%x\n", omap_state->overlay->caps); + } +} + static int omap_plane_atomic_set_property(struct drm_plane *plane, struct drm_plane_state *state, struct drm_property *property, @@ -387,6 +402,7 @@ static const struct drm_plane_funcs omap_plane_funcs = { .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, .atomic_set_property = omap_plane_atomic_set_property, .atomic_get_property = omap_plane_atomic_get_property, + .atomic_print_state = omap_plane_atomic_print_state, }; static bool omap_plane_supports_yuv(struct drm_plane *plane)