On Fri, 6 Jan 2023 at 02:38, Jessica Zhang <quic_jesszhan@xxxxxxxxxxx> wrote: > > > > On 1/5/2023 3:33 AM, Daniel Vetter wrote: > > On Wed, Jan 04, 2023 at 03:40:33PM -0800, Jessica Zhang wrote: > >> Introduce and add support for a solid_fill property. When the solid_fill > >> property is set, and the framebuffer is set to NULL, memory fetch will be > >> disabled. > >> > >> In addition, loosen the NULL FB checks within the atomic commit callstack > >> to allow a NULL FB when the solid_fill property is set and add FB checks > >> in methods where the FB was previously assumed to be non-NULL. > >> > >> Finally, have the DPU driver use drm_plane_state.solid_fill and instead of > >> dpu_plane_state.color_fill, and add extra checks in the DPU atomic commit > >> callstack to account for a NULL FB in cases where solid_fill is set. > >> > >> Some drivers support hardware that have optimizations for solid fill > >> planes. This series aims to expose these capabilities to userspace as > >> some compositors have a solid fill flag (ex. SOLID_COLOR in the Android > >> hardware composer HAL) that can be set by apps like the Android Gears > >> app. > >> > >> Userspace can set the solid_fill property to a blob containing the > >> appropriate version number and solid fill color (in RGB323232 format) and > >> setting the framebuffer to NULL. > >> > >> Note: Currently, there's only one version of the solid_fill blob property. > >> However if other drivers want to support a similar feature, but require > >> more than just the solid fill color, they can extend this feature by > >> creating additional versions of the drm_solid_fill struct. > >> > >> Changes in V2: > >> - Dropped SOLID_FILL_FORMAT property (Simon) > >> - Switched to implementing solid_fill property as a blob (Simon, Dmitry) > >> - Changed to checks for if solid_fill_blob is set (Dmitry) > >> - Abstracted (plane_state && !solid_fill_blob) checks to helper method > >> (Dmitry) > >> - Removed DPU_PLANE_COLOR_FILL_FLAG > >> - Fixed whitespace and indentation issues (Dmitry) > > > > Now that this is a blob, I do wonder again whether it's not cleaner to set > > the blob as the FB pointer. Or create some kind other kind of special data > > source objects (because solid fill is by far not the only such thing). > > > > We'd still end up in special cases like when userspace that doesn't > > understand solid fill tries to read out such a framebuffer, but these > > cases already exist anyway for lack of priviledges. > > > > So I still think that feels like the more consistent way to integrate this > > feature. Which doesn't mean it has to happen like that, but the > > patches/cover letter should at least explain why we don't do it like this. > > Hi Daniel, > > IIRC we were facing some issues with this check [1] when trying to set > FB to a PROP_BLOB instead. Which is why we went with making it a > separate property instead. Will mention this in the cover letter. What kind of issues? Could you please describe them? > > [1] > https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/drm_property.c#L71 > > Thanks, > > Jessica Zhang > > > -Daniel > > > >> > >> Changes in V3: > >> - Fixed some logic errors in atomic checks (Dmitry) > >> - Introduced drm_plane_has_visible_data() and drm_atomic_check_fb() helper > >> methods (Dmitry) > >> > >> Jessica Zhang (3): > >> drm: Introduce solid fill property for drm plane > >> drm: Adjust atomic checks for solid fill color > >> drm/msm/dpu: Use color_fill property for DPU planes > >> > >> drivers/gpu/drm/drm_atomic.c | 136 +++++++++++++--------- > >> drivers/gpu/drm/drm_atomic_helper.c | 34 +++--- > >> drivers/gpu/drm/drm_atomic_state_helper.c | 9 ++ > >> drivers/gpu/drm/drm_atomic_uapi.c | 59 ++++++++++ > >> drivers/gpu/drm/drm_blend.c | 17 +++ > >> drivers/gpu/drm/drm_plane.c | 8 +- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 9 +- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 65 +++++++---- > >> include/drm/drm_atomic_helper.h | 5 +- > >> include/drm/drm_blend.h | 1 + > >> include/drm/drm_plane.h | 62 ++++++++++ > >> 11 files changed, 302 insertions(+), 103 deletions(-) > >> > >> -- > >> 2.38.1 > >> > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- With best wishes Dmitry