On 11/24/2022 12:50 AM, Pekka Paalanen wrote:
On Wed, 23 Nov 2022 15:27:04 -0800
Jessica Zhang wrote:
On 11/9/2022 1:18 AM, Pekka Paalanen wrote:
On Tue, 8 Nov 2022 23:01:47 +0100
Sebastian Wick wrote:
On Tue, Nov 8, 2022 at 7:51 PM Simon Ser wrote:
cc'ing Pekka and wayland-devel for userspace devs feedback on the new uAPI.
Hi all,
thanks! Comments below.
Thanks for the feedback!
On Saturday, October 29th, 2022 at 14:08, 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
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.
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.
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[];
};
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.
Another approach might be using a format modifier instead of the FB flag.
What do you think?
First off, we only need to represent the value of a single pixel here. So I'm
not quite following why we need format modifiers. Format modifiers describe how
pixels are laid out in memory. Since there's a single pixel described, this
is non-sensical to me, the format modifier is always LINEAR.
Agreed.
Then, I can understand why putting the pixel_format in there is tempting to
guarantee future extensibility, but it also adds complexity. For instance, how
does user-space figure out which formats can be used for COLOR_FILL? Can
user-space use any format supported by the plane? What does it mean for
multi-planar formats? Do we really want the kernel to have conversion logic for
all existing formats? Do we need to also add a new read-only blob prop to
indicate supported COLOR_FILL formats?
FWIW the formats supported by solid_fill wouldn't necessarily be all the
formats supported by the plane (ex. for msm/dpu, solid_fill only
supports all RGB color variants, though planes can normally support YUV
formats too).
That being said, I'm ok with having the solid_fill take in only
RGBA32323232 format based on the comments below.
Right. This does not seem to require pixel formats at all.
The point of pixel formats is to be able to feed large amounts of data
as-is into hardware and avoid the CPU ever touching it. You do that
with DRM FBs pointing to suitably allocated hardware buffers. But here
we have exactly one pixel, which I imagine will always be read by the
CPU so the driver will convert it into a hardware-specific format and
program it; probably the driver will not create an internal DRM FB for
it. >
The above might also be a reason to not model this as a special-case
DRM FB in UAPI. Or, at least you need a whole new ioctl to create such
DRM FB to avoid the need to allocate e.g. a dumb buffer or a
GPU-specific buffer. >
What one does need is what Sebastian brought up: does it support alpha
or not?
Hmm, the drm_plane struct already supports an alpha property so it seems
a bit redundant to also have a separate alpha value in the solid fill color.
Hi Jessica,
that's a good point! - Assuming that if hardware supports fill with
alpha, it supports plane-alpha with real FBs as well.
That being said, we could have it so that setting the alpha for the
solid_fill property will also change the value of the plane's alpha
property too.
No! Definitely not. That would be confusing.
One must not have properties that change the value of other
non-immutable properties. It would become a re
