Message-ID: <a9503013-989f-47c7-a879-e862868ea23d@amd.com>
Date:   Tue, 26 Sep 2023 11:25:41 -0400
From:   Harry Wentland <harry.wentland@....com>
To:     Jessica Zhang <quic_jesszhan@...cinc.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Rob Clark <robdclark@...il.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Sean Paul <sean@...rly.run>,
        Marijn Suijten <marijn.suijten@...ainline.org>
Cc:     sebastian.wick@...hat.com, quic_abhinavk@...cinc.com,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        ppaalanen@...il.com, laurent.pinchart@...asonboard.com,
        linux-arm-msm@...r.kernel.org, wayland-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org
Subject: Re: [PATCH RFC v6 00/10] Support for Solid Fill Planes



On 2023-08-28 20:05, Jessica Zhang wrote:
> 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.
> 
> In order to expose this capability to userspace, this series will:
> 
> - Introduce solid_fill and pixel_source properties to allow userspace to
>   toggle between FB and solid fill sources
> - Loosen NULL FB checks within the DRM atomic commit callstack to allow
>   for NULL FB when solid fill is enabled.
> - Add NULL FB checks in methods where FB was previously assumed to be
>   non-NULL
> - Have MSM DPU driver use drm_plane_state.solid_fill instead of
>   dpu_plane_state.color_fill
> 
> Note: The solid fill planes feature depends on both the solid_fill *and*
> pixel_source properties.
> 
> To use this feature, userspace can set the solid_fill property to a blob
> containing the appropriate version number and solid fill color (in
> RGB323232 format) and and setting the pixel_source property to
> DRM_PLANE_PIXEL_SOURCE_COLOR. This will disable memory fetch and the
> resulting plane will display the color specified by the solid_fill blob.
> 
> 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.
> 
> This 2 property approach was chosen because passing in a special 1x1 FB
> with the necessary color information would have unecessary overhead that
> does not reflect the behavior of the solid fill feature. In addition,
> assigning the solid fill blob to FB_ID would require loosening some core
> drm_property checks that might cause unwanted side effects elsewhere.
> 

I didn't have a detailed review of this patchset but at a high-level this
change makes sense to me.

Feel free to add my
Acked-by: Harry Wentland <harry.wentland@....com>
to patches 1-5.

Harry

> ---
> Changes in v6:
> - Have _dpu_plane_color_fill() take in a single ABGR8888 color instead
>   of having separate alpha and BGR color parameters (Dmitry)
> - Drop plane->state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB check
>   in SetPlane ioctl (Dmitry)
> - Add DRM_PLANE_PIXEL_SOURCE_NONE as a default pixel source (Sebastian)
> - Dropped versioning from solid fill property blob (Dmitry)
> - Use DRM_ENUM_NAME_FN (Dmitry)
> - Use drm_atomic_replace_property_blob_from_id() (Dmitry)
> - drm_atomic_check_fb -> drm_atomic_plane_check_fb (Dmitry)
> - Group redundant NULL FB checks (Dmitry)
> - Squashed drm_plane_needs_disable() implementation with 
>   DRM_PLANE_PIXEL_SOURCE_NONE declaration (Sebastian)
> - Add comment to support RGBA solid fill color in the future (Dmitry)
> - Link to v5: https://lore.kernel.org/r/20230728-solid-fill-v5-0-053dbefa909c@quicinc.com
> 
> Changes in v5:
> - Added support for PIXEL_SOURCE_NONE (Sebastian)
> - Added WARN_ON() in drm_plane_has_visible_data() if pixel_source isn't
>   set (Dmitry)
> - Added debugfs support for both properties (Dmitry)
> - Corrected u32 to u8 conversion (Pekka)
> - Moved drm_solid_fill_info struct and related documentation to
>   include/uapi (Pekka)
> - Changed drm_solid_fill_info.version to __u32 for data alignment (Pekka)
> - Added more detailed UAPI and kernel documentation (Pekka)
> - Reordered patch series so that the pixel_source property is introduced
>   before solid_fill (Dmitry)
> - Fixed inconsistent ABGR8888/RGBA8888 format declaration (Pekka)
> - Reset pixel_source to FB in drm_mode_setplane() (Dmitry)
> - Rename supported_sources to extra_sources (Dmitry)
> - Only destroy old solid_fill blob state if new state is valid (Pekka)
> - Link to v4: https://lore.kernel.org/r/20230404-solid-fill-v4-0-f4ec0caa742d@quicinc.com
> 
> Changes in v4:
> - Rebased onto latest kernel
> - Reworded cover letter for clarity (Dmitry)
> - Reworded commit messages for clarity
> - Split existing changes into smaller commits
> - Added pixel_source enum property (Dmitry, Pekka, Ville)
> - Updated drm-kms comment docs with pixel_source and solid_fill
>   properties (Dmitry)
> - Inlined drm_atomic_convert_solid_fill_info() (Dmitry)
> - Passed in plane state alpha value to _dpu_plane_color_fill_pipe()
> - Link to v3: https://lore.kernel.org/r/20230104234036.636-1-quic_jesszhan@quicinc.com
> 
> 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)
> - Fixed typo in drm_solid_fill struct documentation
> - Created drm_plane_has_visible_data() helper and corrected CRTC and FB
>   NULL-check logic (Dmitry)
> - Merged `if (fb)` blocks in drm_atomic_plane_check() and abstracted
>   them into helper method (Dmitry)
> - Inverted `if (solid_fill_enabled) else if (fb)` check order (Dmitry)
> - Fixed indentation (Dmitry)
> 
> Changes in v2:
> - Dropped SOLID_FILL_FORMAT property (Simon)
> - Switched to implementing solid_fill property as a blob (Simon, Dmitry)
> - Added drm_solid_fill and drm_solid_fill_info structs (Simon)
> - 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)
> - Changed to checks for if solid_fill_blob is set (Dmitry)
> - Abstracted (plane_state && !solid_fill_blob) checks to helper method
>   (Dmitry)
> - Fixed dropped 'const' warning
> - Added helper to convert color fill to BGR888 (Rob)
> - Fixed indentation issue (Dmitry)
> - Added support for solid fill on planes of varying sizes
> 
> ---
> Jessica Zhang (10):
>       drm: Introduce pixel_source DRM plane property
>       drm: Introduce solid fill DRM plane property
>       drm: Add solid fill pixel source
>       drm/atomic: Add pixel source to plane state dump
>       drm/atomic: Add solid fill data to plane state dump
>       drm/atomic: Move framebuffer checks to helper
>       drm/atomic: Loosen FB atomic checks
>       drm/msm/dpu: Allow NULL FBs in atomic commit
>       drm/msm/dpu: Use DRM solid_fill property
>       drm/msm/dpu: Add solid fill and pixel source properties
> 
>  drivers/gpu/drm/drm_atomic.c              | 147 +++++++++++++++++-------------
>  drivers/gpu/drm/drm_atomic_helper.c       |  36 ++++----
>  drivers/gpu/drm/drm_atomic_state_helper.c |  10 ++
>  drivers/gpu/drm/drm_atomic_uapi.c         |  30 ++++++
>  drivers/gpu/drm/drm_blend.c               | 129 ++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_crtc_internal.h       |   1 +
>  drivers/gpu/drm/drm_plane.c               |  27 +++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |   9 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c |  80 ++++++++++------
>  include/drm/drm_atomic_helper.h           |   4 +-
>  include/drm/drm_blend.h                   |   3 +
>  include/drm/drm_plane.h                   |  90 ++++++++++++++++++
>  include/uapi/drm/drm_mode.h               |  24 +++++
>  13 files changed, 478 insertions(+), 112 deletions(-)
> ---
> base-commit: 00ee72279c963989ab435b0bc90b5dc05a9aab79
> change-id: 20230404-solid-fill-05016175db36
> 
> Best regards,

Powered by blists - more mailing lists