Re: [RFC PATCH v3 0/3] Support for Solid Fill Planes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/5/2023 7:43 PM, Dmitry Baryshkov wrote:
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?

Hi Dmitry,

PROP_BLOB is defined as a legacy type here [1], but FB_ID is a PROP_OBJECT which is defined as an extended type [2]. So, setting a property blob as the FB would fail drm_property_flags_valid() due to this check [3].

[1] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/include/uapi/drm/drm_mode.h#L523

[2] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/include/uapi/drm/drm_mode.h#L534

[3] https://gitlab.freedesktop.org/drm/msm/-/blob/msm-next/drivers/gpu/drm/drm_property.c#L71

Thanks,

Jessica Zhang



[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



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux