On 27/10/2021 14:23, Tomi Valkeinen wrote:
> On 18/10/2021 17:28, Neil Armstrong wrote:
>> From: Benoit Parrot <bpar...@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 <bpar...@ti.com>
>> Signed-off-by: Neil Armstrong <narmstr...@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
