On 10/29/2022 5:08 AM, Dmitry Baryshkov wrote:
On 29/10/2022 01:59, Jessica Zhang wrote:
Add support for COLOR_FILL and COLOR_FILL_FORMAT properties for
drm_plane. In addition, add support for setting and getting the values
of these properties.
COLOR_FILL represents the color fill of a plane while COLOR_FILL_FORMAT
represents the format of the color fill. Userspace can set enable solid
fill on a plane by assigning COLOR_FILL to a uint64_t value, assigning
the COLOR_FILL_FORMAT property to a uint32_t value, and setting the
framebuffer to NULL.
Signed-off-by: Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> >
Planes report supported formats using the drm_mode_getplane(). You'd
also need to tell userspace, which formats are supported for color fill.
I don't think one supports e.g. YV12.
Hey Dmitry,
Good point. Forgot to add this in the patch [3/3] commit message, but
FWIW MSM DPU devices only support the RGB format variants [1].
[1]
https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c#L697
A bit of generic comment for the discussion (this is an RFC anyway).
Using color_fill/color_fill_format properties sounds simple, but this
might be not generic enough. Limiting color_fill to 32 bits would
prevent anybody from using floating point formats (e.g.
DRM_FORMAT_XRGB16161616F, 64-bit value). Yes, this can be solved with
e.g. using 64-bit for the color_fill value, but then this doesn't sound
extensible too much.
Hm... I can definitely change color_fill to use u64 instead. As for
making it more extensible, do you have any suggestions?
So, a question for other hardware maintainers. Do we have hardware that
supports such 'color filled' planes? Do we want to support format
modifiers for filling color/data? Because what I have in mind is closer
to the blob structure, which can then be used for filling the plane:
struct color_fill_blob {
u32 pixel_format;
u64 modifiers4];
u32 pixel_data_size; // fixme: is this necessary?
u8 pixel_data[];
};
Just a question about this blob struct -- what is the purpose of pixel_data?
At least for devices using the DPU driver, the only data needed to
enable solid fill is color_fill and color_fill_format. I'd also be
interested in knowing if there are other drivers support a similar
feature and what is needed for them.
And then... This sounds a lot like a custom framebuffer.
So, maybe what should we do instead is to add new DRM_MODE_FB_COLOR_FILL
flag to the framebuffers, which would e.g. mean that the FB gets stamped
all over the plane. This would also save us from changing if (!fb)
checks all over the drm core.
JFYI we did originally consider using a custom 1x1 FB to for color fill
[1], but decided to go with a plane property instead. IIRC the
conclusion was that having color fill as a plane property is a cleaner
solution.
As for creating a new blob struct to hold all color fill info, I'm open
to implementing that over having 2 separate properties.
[1] https://oftc.irclog.whitequark.org/dri-devel/2022-09-23#31409842
Another approach might be using a format modifier instead of the FB flag.
I like the idea of having a format modifier denoting if the driver
supports color_fill for that specific format. This would allow userspace
to know which formats are supported by solid fill planes.
Thanks,
Jessica Zhang
What do you think?
--
With best wishes
Dmitry
