| Message ID | 20230404-solid-fill-v4-1-f4ec0caa742d@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support for Solid Fill Planes | expand |
On 30/06/2023 03:25, Jessica Zhang wrote: > Document and add support for solid_fill property to drm_plane. In > addition, add support for setting and getting the values for solid_fill. > > To enable solid fill planes, userspace must assign a property blob to > the "solid_fill" plane property containing the following information: > > struct drm_solid_fill_info { > u8 version; > u32 r, g, b; > }; > > Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 9 +++++ > drivers/gpu/drm/drm_atomic_uapi.c | 55 +++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_blend.c | 33 +++++++++++++++++++ > include/drm/drm_blend.h | 1 + > include/drm/drm_plane.h | 43 ++++++++++++++++++++++++ > 5 files changed, 141 insertions(+) Also, I think the point which we missed up to now. Could you please add both new properties to dri/N/state debugfs?
On 6/30/2023 3:33 AM, Dmitry Baryshkov wrote: > On 30/06/2023 03:25, Jessica Zhang wrote: >> Document and add support for solid_fill property to drm_plane. In >> addition, add support for setting and getting the values for solid_fill. >> >> To enable solid fill planes, userspace must assign a property blob to >> the "solid_fill" plane property containing the following information: >> >> struct drm_solid_fill_info { >> u8 version; >> u32 r, g, b; >> }; >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >> --- >> drivers/gpu/drm/drm_atomic_state_helper.c | 9 +++++ >> drivers/gpu/drm/drm_atomic_uapi.c | 55 >> +++++++++++++++++++++++++++++++ >> drivers/gpu/drm/drm_blend.c | 33 +++++++++++++++++++ >> include/drm/drm_blend.h | 1 + >> include/drm/drm_plane.h | 43 ++++++++++++++++++++++++ >> 5 files changed, 141 insertions(+) > > Also, I think the point which we missed up to now. Could you please add > both new properties to dri/N/state debugfs? Hi Dmitry, Good catch -- acked. Thanks, Jessica Zhang > > -- > With best wishes > Dmitry >
On 6/30/2023 1:27 AM, Pekka Paalanen wrote: > On Thu, 29 Jun 2023 17:25:00 -0700 > Jessica Zhang <quic_jesszhan@quicinc.com> wrote: > >> Document and add support for solid_fill property to drm_plane. In >> addition, add support for setting and getting the values for solid_fill. >> >> To enable solid fill planes, userspace must assign a property blob to >> the "solid_fill" plane property containing the following information: >> >> struct drm_solid_fill_info { >> u8 version; >> u32 r, g, b; >> }; >> >> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> > > Hi Jessica, > > I've left some general UAPI related comments here. > >> --- >> drivers/gpu/drm/drm_atomic_state_helper.c | 9 +++++ >> drivers/gpu/drm/drm_atomic_uapi.c | 55 +++++++++++++++++++++++++++++++ >> drivers/gpu/drm/drm_blend.c | 33 +++++++++++++++++++ >> include/drm/drm_blend.h | 1 + >> include/drm/drm_plane.h | 43 ++++++++++++++++++++++++ >> 5 files changed, 141 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c >> index 784e63d70a42..fe14be2bd2b2 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 d867e7f9f2cd..a28b4ee79444 100644 >> --- a/drivers/gpu/drm/drm_atomic_uapi.c >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c >> @@ -316,6 +316,51 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, >> } >> EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector); >> >> +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; > > Is it ok to destroy old state before it is guaranteed that the new > state is accepted? Hi Pekka, Good point. I'll change this behavior so that an error case will keep the old solid_fill_blob value. > >> + >> + memset(&state->solid_fill, 0, sizeof(state->solid_fill)); >> + >> + if (blob) { >> + struct drm_solid_fill_info *user_info = (struct drm_solid_fill_info *)blob->data; >> + >> + 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 = user_info->version; >> + >> + /* Add more versions if necessary */ >> + if (blob_version == 1) { >> + state->solid_fill.r = user_info->r; >> + state->solid_fill.g = user_info->g; >> + state->solid_fill.b = user_info->b; >> + } 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; > > Btw. how does the atomic check machinery work here? > > I expect that a TEST_ONLY atomic commit will do all the above checks > and return failure if anything is not right. Right? Correct, drm_atomic_set_property() will still be called for a TEST_ONLY commit, so these checks will still happen and return an error (or set the property) when appropriate. > >> + } >> + 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 +589,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 +668,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 6e74de833466..38c3c5d6453a 100644 >> --- a/drivers/gpu/drm/drm_blend.c >> +++ b/drivers/gpu/drm/drm_blend.c >> @@ -185,6 +185,10 @@ >> * plane does not expose the "alpha" property, then this is >> * assumed to be 1.0 >> * >> + * solid_fill: >> + * solid_fill is set up with drm_plane_create_solid_fill_property(). It >> + * contains pixel data that drivers can use to fill a plane. > > This is a nice start, but I feel it needs to explain much more about > how userspace should go about making use of this. > > Yeah, the pixel_source property comes in the next patch, but I feel > that it is harder to review if the doc is built over multiple patches. > My personal approach would be to write the doc in full and referring to > pixel_source property already, and explain in the commit message that > the next patch will add pixel_source so people don't wonder about > referring to a non-existing property. > > I mean just a reference to pixel_source, and leave the actual > pixel_source doc for the patch adding the property like it already is. > > Dmitry's suggestion of swapping the patch order is good too. Makes sense. I'll switch the patch order, but will keep this in mind. > >> + * >> * Note that all the property extensions described here apply either to the >> * plane or the CRTC (e.g. for the background color, which currently is not >> * exposed and assumed to be black). >> @@ -615,3 +619,32 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane, >> return 0; >> } >> EXPORT_SYMBOL(drm_plane_create_blend_mode_property); >> + >> +/** >> + * drm_plane_create_solid_fill_property - create a new solid_fill property >> + * @plane: drm plane >> + * >> + * This creates a new property that holds pixel data for solid fill planes. This >> + * property is exposed to userspace as a property blob called "solid_fill". >> + * >> + * For information on what the blob contains, see `drm_solid_fill_info`. > > I think you should be more explicit here. For example: the blob must > contain exactly one struct drm_solid_fill_info. > > It's better to put this content spec with the UAPI doc rather than in this > kerner-internal function doc that userspace programmers won't know to > look at. Acked. > >> + * >> + * Returns: >> + * Zero on success, negative errno on failure. >> + */ >> +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 51291983ea44..f6ab313cb83e 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; >> +}; > > Shouldn't UAPI structs be in UAPI headers? Acked, will move this to uapi/drm_mode.h > > Shouldn't UAPI structs use explicit padding to not leave any gaps when > it's unavoidable? And the kernel to check that the gaps are indeed > zeroed? I don't believe so... From my understanding, padding will be taken care of by the compiler by default. Looking at the drm_mode_modeinfo UAPI struct [1], it also doesn't seem to do explicit padding. And the corresponding set_property() code doesn't seem check the gaps either. That being said, it's possible that I'm missing something here, so please let me know if that's the case. [1] https://elixir.bootlin.com/linux/v6.5-rc1/source/include/uapi/drm/drm_mode.h#L242 > > It also needs more UAPI doc, like a link to the property doc that uses > this and an explanation of what the values mean. Acked. Thanks, Jessica Zhang > > > Thanks, > pq > >> + >> +/** >> + * 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 >> >
On 7/11/2023 12:42 AM, Pekka Paalanen wrote: > On Mon, 10 Jul 2023 16:12:06 -0700 > Jessica Zhang <quic_jesszhan@quicinc.com> wrote: > >> On 6/30/2023 1:27 AM, Pekka Paalanen wrote: >>> On Thu, 29 Jun 2023 17:25:00 -0700 >>> Jessica Zhang <quic_jesszhan@quicinc.com> wrote: >>> >>>> Document and add support for solid_fill property to drm_plane. In >>>> addition, add support for setting and getting the values for solid_fill. >>>> >>>> To enable solid fill planes, userspace must assign a property blob to >>>> the "solid_fill" plane property containing the following information: >>>> >>>> struct drm_solid_fill_info { >>>> u8 version; >>>> u32 r, g, b; >>>> }; >>>> >>>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> >>> >>> Hi Jessica, >>> >>> I've left some general UAPI related comments here. >>> >>>> --- >>>> drivers/gpu/drm/drm_atomic_state_helper.c | 9 +++++ >>>> drivers/gpu/drm/drm_atomic_uapi.c | 55 +++++++++++++++++++++++++++++++ >>>> drivers/gpu/drm/drm_blend.c | 33 +++++++++++++++++++ >>>> include/drm/drm_blend.h | 1 + >>>> include/drm/drm_plane.h | 43 ++++++++++++++++++++++++ >>>> 5 files changed, 141 insertions(+) > > ... > >>>> 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 51291983ea44..f6ab313cb83e 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; >>>> +}; >>> >>> Shouldn't UAPI structs be in UAPI headers? >> >> Acked, will move this to uapi/drm_mode.h >> >>> >>> Shouldn't UAPI structs use explicit padding to not leave any gaps when >>> it's unavoidable? And the kernel to check that the gaps are indeed >>> zeroed? >> >> I don't believe so... From my understanding, padding will be taken care >> of by the compiler by default. Looking at the drm_mode_modeinfo UAPI >> struct [1], it also doesn't seem to do explicit padding. And the >> corresponding set_property() code doesn't seem check the gaps either. >> >> That being said, it's possible that I'm missing something here, so >> please let me know if that's the case. >> >> [1] >> https://elixir.bootlin.com/linux/v6.5-rc1/source/include/uapi/drm/drm_mode.h#L242 > > I suspect that drm_mode_modeinfo predates the lessons learnt about > "botching up ioctls" by many years: > https://www.kernel.org/doc/Documentation/ioctl/botching-up-ioctls.rst > > drm_mode_modeinfo goes all the way back to > > commit f453ba0460742ad027ae0c4c7d61e62817b3e7ef > Date: Fri Nov 7 14:05:41 2008 -0800 > > DRM: add mode setting support > > and > > commit e0c8463a8b00b467611607df0ff369d062528875 > Date: Fri Dec 19 14:50:50 2008 +1000 > > drm: sanitise drm modesetting API + remove unused hotplug > > and it got the proper types later in > > commit 1d7f83d5ad6c30b385ba549c1c3a287cc872b7ae > Date: Thu Feb 26 00:51:42 2009 +0100 > > make drm headers use strict integer types > > > My personal feeling is that if you cannot avoid padding in a struct, > convert it into explicit fields anyway and require them to be zero. > That way if you ever need to extend or modify the struct, you already > have an "unused" field that old userspace guarantees to be zero, so you > can re-purpose it when it's not zero. > > A struct for blob contents is maybe needing slightly less forward > planning than ioctl struct, because KMS properties are cheap compared > to ioctl numbers, I believe. > > Maybe eliminating compiler induced padding in structs is not strictly > necessary, but it seems like a good idea to me, because compilers are > allowed to leave the padding bits undefined. If a struct was filled in > by the kernel and delivered to userspace, undefined padding could even > be a security leak, theoretically. > > Anyway, don't take my word for it. Maybe kernel devs have a different > style. Ah, got it. Thanks for the info! Looking over more recent implementations of blob properties, I do see that there's a precedent for explicit padding [1]. I think I could also just make `version` a __u32 instead. Either way, that seems to be how other structs declare `version`. Thanks, Jessica Zhang [1] https://elixir.bootlin.com/linux/latest/source/include/uapi/drm/virtgpu_drm.h#L178 > > > Thanks, > pq > >>> >>> It also needs more UAPI doc, like a link to the property doc that uses >>> this and an explanation of what the values mean. >> >> Acked. >> >> Thanks, >> >> Jessica Zhang >> >>> >>> >>> Thanks, >>> pq >>> >>>> + >>>> +/** >>>> + * 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 >>>> >>> >
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index 784e63d70a42..fe14be2bd2b2 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 d867e7f9f2cd..a28b4ee79444 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -316,6 +316,51 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, } EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector); +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) { + struct drm_solid_fill_info *user_info = (struct drm_solid_fill_info *)blob->data; + + 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 = user_info->version; + + /* Add more versions if necessary */ + if (blob_version == 1) { + state->solid_fill.r = user_info->r; + state->solid_fill.g = user_info->g; + state->solid_fill.b = user_info->b; + } 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 +589,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 +668,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 6e74de833466..38c3c5d6453a 100644 --- a/drivers/gpu/drm/drm_blend.c +++ b/drivers/gpu/drm/drm_blend.c @@ -185,6 +185,10 @@ * plane does not expose the "alpha" property, then this is * assumed to be 1.0 * + * solid_fill: + * solid_fill is set up with drm_plane_create_solid_fill_property(). It + * contains pixel data that drivers can use to fill a plane. + * * Note that all the property extensions described here apply either to the * plane or the CRTC (e.g. for the background color, which currently is not * exposed and assumed to be black). @@ -615,3 +619,32 @@ int drm_plane_create_blend_mode_property(struct drm_plane *plane, return 0; } EXPORT_SYMBOL(drm_plane_create_blend_mode_property); + +/** + * drm_plane_create_solid_fill_property - create a new solid_fill property + * @plane: drm plane + * + * This creates a new property that holds pixel data for solid fill planes. This + * property is exposed to userspace as a property blob called "solid_fill". + * + * For information on what the blob contains, see `drm_solid_fill_info`. + * + * Returns: + * Zero on success, negative errno on failure. + */ +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 51291983ea44..f6ab313cb83e 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
Document and add support for solid_fill property to drm_plane. In addition, add support for setting and getting the values for solid_fill. To enable solid fill planes, userspace must assign a property blob to the "solid_fill" plane property containing the following information: struct drm_solid_fill_info { u8 version; u32 r, g, b; }; Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com> --- drivers/gpu/drm/drm_atomic_state_helper.c | 9 +++++ drivers/gpu/drm/drm_atomic_uapi.c | 55 +++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_blend.c | 33 +++++++++++++++++++ include/drm/drm_blend.h | 1 + include/drm/drm_plane.h | 43 ++++++++++++++++++++++++ 5 files changed, 141 insertions(+)