[RFC PATCH 1/3] drm: Introduce color fill properties for drm plane
Jessica Zhang
quic_jesszhan at quicinc.com
Mon Oct 31 22:24:48 UTC 2022
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 at quicinc.com> >
> 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
>
More information about the dri-devel
mailing list