[RFC PATCH v3 1/3] drm: Introduce solid fill property for drm plane
Jessica Zhang
quic_jesszhan at quicinc.com
Thu Jan 19 16:24:40 UTC 2023
On 1/19/2023 7:57 AM, Harry Wentland wrote:
>
>
> On 1/18/23 17:53, Jessica Zhang wrote:
>>
>>
>> On 1/18/2023 10:57 AM, Harry Wentland wrote:
>>> On 1/4/23 18:40, Jessica Zhang wrote:
>>>> Add support for solid_fill property to drm_plane. In addition, add
>>>> support for setting and getting the values for solid_fill.
>>>>
>>>> solid_fill holds data for supporting solid fill planes. The property
>>>> accepts an RGB323232 value and the driver data is formatted as such:
>>>>
>>>> struct drm_solid_fill {
>>>> u32 r;
>>>> u32 g;
>>>> u32 b;
>>>> };
>>>
>>> Rather than special-casing this would it make sense to define this
>>> as a single pixel of a FOURCC property?
>>>
>>> I.e., something like this:
>>>
>>> struct drm_solid_fill_info {
>>> u32 format; /* FOURCC value */
>>> u64 value; /* FOURCC pixel value */
>>> }
>>>
>>> That removes some ambiguity how the value should be interpreted, i.e.,
>>> it can be interpreted like a single pixel of the specified FOURCC format.
>>>
>>> It might also make sense to let drivers advertise the supported
>>> FOURCC formats for solid_fill planes.
>>
>> Hi Harry,
>>
>> The initial v1 of this RFC had support for taking in a format and such, but it was decided that just supporting RGB323232 would work too.
>>
>> Here's the original thread discussing it [1], but to summarize, the work needed to convert the solid fill color to RGB is trivial (as it's just a single pixel of data). In addition, supporting various formats for solid_fill would add complexity as we'd have to communicate which formats are supported.
>>
>> [1] https://lists.freedesktop.org/archives/dri-devel/2022-November/379148.html
>>
>
> Make sense, and thanks for summarizing.
>
> The only comment I would have left then, is that maybe it'd be good to add
> an alpha value. I think it was suggested by someone else as well.
Yep it was mentioned in the v1 discussion, but we came to the conclusion
that user can set the alpha through the separate alpha plane property.
Thanks,
Jessica Zhang
>
>>>
>>> Is there an implementation for this in a corresponding canonical
>>> upstream userspace project, to satisfy [1]? If not, what is the plan
>>> for this? If so, please point to the corresponding patches.
>>
>> The use case we're trying to support here is the Android HWC SOLID_FILL hint [1], though it can also be used to address the Wayland single pixel FB protocol [2]. I'm also planning to add an IGT test to show an example of end to end usage.
>>
>> [1] https://android.googlesource.com/platform/hardware/interfaces/+/refs/heads/master/graphics/composer/aidl/android/hardware/graphics/composer3/Composition.aidl#52
>>
>> [2] https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/104
>>
>
> Makes sense.
>
> Harry
>
>> Thanks,
>>
>> Jessica Zhang
>>
>>>
>>> [1] https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
>>>
>>> Harry
>>>
>>>>
>>>> To enable solid fill planes, userspace must assigned solid_fill to a
>>>> property blob containing the following information:
>>>>
>>>> struct drm_solid_fill_info {
>>>> u8 version;
>>>> u32 r, g, b;
>>>> };
>>>>
>>>> Changes in V2:
>>>> - Changed solid_fill property to a property blob (Simon, Dmitry)
>>>> - Added drm_solid_fill struct (Simon)
>>>> - Added drm_solid_fill_info struct (Simon)
>>>>
>>>> Changes in V3:
>>>> - Corrected typo in drm_solid_fill struct documentation
>>>>
>>>> Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
>>>> ---
>>>> drivers/gpu/drm/drm_atomic_state_helper.c | 9 ++++
>>>> drivers/gpu/drm/drm_atomic_uapi.c | 59 +++++++++++++++++++++++
>>>> drivers/gpu/drm/drm_blend.c | 17 +++++++
>>>> include/drm/drm_blend.h | 1 +
>>>> include/drm/drm_plane.h | 43 +++++++++++++++++
>>>> 5 files changed, 129 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
>>>> index dfb57217253b..c96fd1f2ad99 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
>>>> @@ -253,6 +253,11 @@ void __drm_atomic_helper_plane_state_reset(struct drm_plane_state *plane_state,
>>>> plane_state->alpha = DRM_BLEND_ALPHA_OPAQUE;
>>>> plane_state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
>>>> + if (plane_state->solid_fill_blob) {
>>>> + drm_property_blob_put(plane_state->solid_fill_blob);
>>>> + plane_state->solid_fill_blob = NULL;
>>>> + }
>>>> +
>>>> if (plane->color_encoding_property) {
>>>> if (!drm_object_property_get_default_value(&plane->base,
>>>> plane->color_encoding_property,
>>>> @@ -335,6 +340,9 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>>>> if (state->fb)
>>>> drm_framebuffer_get(state->fb);
>>>> + if (state->solid_fill_blob)
>>>> + drm_property_blob_get(state->solid_fill_blob);
>>>> +
>>>> state->fence = NULL;
>>>> state->commit = NULL;
>>>> state->fb_damage_clips = NULL;
>>>> @@ -384,6 +392,7 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
>>>> drm_crtc_commit_put(state->commit);
>>>> drm_property_blob_put(state->fb_damage_clips);
>>>> + drm_property_blob_put(state->solid_fill_blob);
>>>> }
>>>> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>>>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>>>> index c06d0639d552..8a1d2fb7a757 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>>> @@ -316,6 +316,55 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
>>>> }
>>>> EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
>>>> +static void drm_atomic_convert_solid_fill_info(struct drm_solid_fill *out,
>>>> + struct drm_solid_fill_info *in)
>>>> +{
>>>> + out->r = in->r;
>>>> + out->g = in->g;
>>>> + out->b = in->b;
>>>> +}
>>>> +
>>>> +static int drm_atomic_set_solid_fill_prop(struct drm_plane_state *state,
>>>> + struct drm_property_blob *blob)
>>>> +{
>>>> + int ret = 0;
>>>> + int blob_version;
>>>> +
>>>> + if (blob == state->solid_fill_blob)
>>>> + return 0;
>>>> +
>>>> + drm_property_blob_put(state->solid_fill_blob);
>>>> + state->solid_fill_blob = NULL;
>>>> +
>>>> + memset(&state->solid_fill, 0, sizeof(state->solid_fill));
>>>> +
>>>> + if (blob) {
>>>> + if (blob->length != sizeof(struct drm_solid_fill_info)) {
>>>> + drm_dbg_atomic(state->plane->dev,
>>>> + "[PLANE:%d:%s] bad solid fill blob length: %zu\n",
>>>> + state->plane->base.id, state->plane->name,
>>>> + blob->length);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + blob_version = ((struct drm_solid_fill_info *)blob->data)->version;
>>>> +
>>>> + /* Append with more versions if necessary */
>>>> + if (blob_version == 1) {
>>>> + drm_atomic_convert_solid_fill_info(&state->solid_fill, blob->data);
>>>> + } else {
>>>> + drm_dbg_atomic(state->plane->dev,
>>>> + "[PLANE:%d:%s] failed to set solid fill (ret=%d)\n",
>>>> + state->plane->base.id, state->plane->name,
>>>> + ret);
>>>> + return -EINVAL;
>>>> + }
>>>> + state->solid_fill_blob = drm_property_blob_get(blob);
>>>> + }
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> static void set_out_fence_for_crtc(struct drm_atomic_state *state,
>>>> struct drm_crtc *crtc, s32 __user *fence_ptr)
>>>> {
>>>> @@ -544,6 +593,13 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
>>>> state->src_w = val;
>>>> } else if (property == config->prop_src_h) {
>>>> state->src_h = val;
>>>> + } else if (property == plane->solid_fill_property) {
>>>> + struct drm_property_blob *solid_fill = drm_property_lookup_blob(dev, val);
>>>> +
>>>> + ret = drm_atomic_set_solid_fill_prop(state, solid_fill);
>>>> + drm_property_blob_put(solid_fill);
>>>> +
>>>> + return ret;
>>>> } else if (property == plane->alpha_property) {
>>>> state->alpha = val;
>>>> } else if (property == plane->blend_mode_property) {
>>>> @@ -616,6 +672,9 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
>>>> *val = state->src_w;
>>>> } else if (property == config->prop_src_h) {
>>>> *val = state->src_h;
>>>> + } else if (property == plane->solid_fill_property) {
>>>> + *val = state->solid_fill_blob ?
>>>> + state->solid_fill_blob->base.id : 0;
>>>> } else if (property == plane->alpha_property) {
>>>> *val = state->alpha;
>>>> } else if (property == plane->blend_mode_property) {
>>>> diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
>>>> index b4c8cab7158c..17ab645c8309 100644
>>>> --- a/drivers/gpu/drm/drm_blend.c
>>>> +++ b/drivers/gpu/drm/drm_blend.c
>>>> @@ -616,3 +616,20 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane,
>>>> return 0;
>>>> }
>>>> EXPORT_SYMBOL(drm_plane_create_blend_mode_property);
>>>> +
>>>> +int drm_plane_create_solid_fill_property(struct drm_plane *plane)
>>>> +{
>>>> + struct drm_property *prop;
>>>> +
>>>> + prop = drm_property_create(plane->dev,
>>>> + DRM_MODE_PROP_ATOMIC | DRM_MODE_PROP_BLOB,
>>>> + "solid_fill", 0);
>>>> + if (!prop)
>>>> + return -ENOMEM;
>>>> +
>>>> + drm_object_attach_property(&plane->base, prop, 0);
>>>> + plane->solid_fill_property = prop;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(drm_plane_create_solid_fill_property);
>>>> diff --git a/include/drm/drm_blend.h b/include/drm/drm_blend.h
>>>> index 88bdfec3bd88..0338a860b9c8 100644
>>>> --- a/include/drm/drm_blend.h
>>>> +++ b/include/drm/drm_blend.h
>>>> @@ -58,4 +58,5 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
>>>> struct drm_atomic_state *state);
>>>> int drm_plane_create_blend_mode_property(struct drm_plane *plane,
>>>> unsigned int supported_modes);
>>>> +int drm_plane_create_solid_fill_property(struct drm_plane *plane);
>>>> #endif
>>>> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
>>>> index 447e664e49d5..3b9da06f358b 100644
>>>> --- a/include/drm/drm_plane.h
>>>> +++ b/include/drm/drm_plane.h
>>>> @@ -40,6 +40,25 @@ enum drm_scaling_filter {
>>>> DRM_SCALING_FILTER_NEAREST_NEIGHBOR,
>>>> };
>>>> +/**
>>>> + * struct drm_solid_fill_info - User info for solid fill planes
>>>> + */
>>>> +struct drm_solid_fill_info {
>>>> + __u8 version;
>>>> + __u32 r, g, b;
>>>> +};
>>>> +
>>>> +/**
>>>> + * struct solid_fill_property - RGB values for solid fill plane
>>>> + *
>>>> + * Note: This is the V1 for this feature
>>>> + */
>>>> +struct drm_solid_fill {
>>>> + uint32_t r;
>>>> + uint32_t g;
>>>> + uint32_t b;
>>>> +};
>>>> +
>>>> /**
>>>> * struct drm_plane_state - mutable plane state
>>>> *
>>>> @@ -116,6 +135,23 @@ struct drm_plane_state {
>>>> /** @src_h: height of visible portion of plane (in 16.16) */
>>>> uint32_t src_h, src_w;
>>>> + /**
>>>> + * @solid_fill_blob:
>>>> + *
>>>> + * Blob containing relevant information for a solid fill plane
>>>> + * including pixel format and data. See
>>>> + * drm_plane_create_solid_fill_property() for more details.
>>>> + */
>>>> + struct drm_property_blob *solid_fill_blob;
>>>> +
>>>> + /**
>>>> + * @solid_fill:
>>>> + *
>>>> + * Pixel data for solid fill planes. See
>>>> + * drm_plane_create_solid_fill_property() for more details.
>>>> + */
>>>> + struct drm_solid_fill solid_fill;
>>>> +
>>>> /**
>>>> * @alpha:
>>>> * Opacity of the plane with 0 as completely transparent and 0xffff as
>>>> @@ -699,6 +735,13 @@ struct drm_plane {
>>>> */
>>>> struct drm_plane_state *state;
>>>> + /*
>>>> + * @solid_fill_property:
>>>> + * Optional solid_fill property for this plane. See
>>>> + * drm_plane_create_solid_fill_property().
>>>> + */
>>>> + struct drm_property *solid_fill_property;
>>>> +
>>>> /**
>>>> * @alpha_property:
>>>> * Optional alpha property for this plane. See
>>>
>
More information about the dri-devel
mailing list