RE: [RFC PATCH v3 00/23] Color Pipeline API w/ VKMS

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

 




> -----Original Message-----
> From: Harry Wentland <harry.wentland@xxxxxxx>
> Sent: Wednesday, November 8, 2023 10:06 PM
> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Cc: wayland-devel@xxxxxxxxxxxxxxxxxxxxx; Harry Wentland
> <harry.wentland@xxxxxxx>; Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>; Pekka
> Paalanen <pekka.paalanen@xxxxxxxxxxxxx>; Simon Ser <contact@xxxxxxxxxxx>;
> Melissa Wen <mwen@xxxxxxxxxx>; Jonas Ådahl <jadahl@xxxxxxxxxx>; Sebastian
> Wick <sebastian.wick@xxxxxxxxxx>; Shashank Sharma
> <shashank.sharma@xxxxxxx>; Alexander Goins <agoins@xxxxxxxxxx>; Joshua
> Ashton <joshua@xxxxxxxxx>; Michel Dänzer <mdaenzer@xxxxxxxxxx>; Aleix Pol
> <aleixpol@xxxxxxx>; Xaver Hugl <xaver.hugl@xxxxxxxxx>; Victoria Brekenfeld
> <victoria@xxxxxxxxxxxx>; Sima <daniel@xxxxxxxx>; Shankar, Uma
> <uma.shankar@xxxxxxxxx>; Naseer Ahmed <quic_naseer@xxxxxxxxxxx>;
> Christopher Braga <quic_cbraga@xxxxxxxxxxx>; Abhinav Kumar
> <quic_abhinavk@xxxxxxxxxxx>; Arthur Grillo <arthurgrillo@xxxxxxxxxx>; Hector
> Martin <marcan@xxxxxxxxx>; Liviu Dudau <Liviu.Dudau@xxxxxxx>; Sasha
> McIntosh <sashamcintosh@xxxxxxxxxx>
> Subject: [RFC PATCH v3 00/23] Color Pipeline API w/ VKMS
> 
> This is an RFC set for a color pipeline API, along with a sample implementation in
> VKMS. All the key API bits are here. VKMS now supports two named transfer
> function colorops and two matrix colorops. We have IGT tests that check all four
> of these colorops with a pixel-by-pixel comparison that checks that these colorops
> do what we expect them to do with a +/- 1 8 bpc code point margin.
> 
> <TODO update below>
> 
> This patchset is grouped as follows:
>  - Patches 1-2: couple general patches/fixes
>  - Patches 3-5: introduce kunit to VKMS
>  - Patch 6: description of motivation and details behind the
>             Color Pipeline API. If you're reading nothing else
>             but are interested in the topic I highly recommend
>             you take a look at this.
>  - Patches 7-23: DRM core and VKMS changes for color pipeline API

Hi Harry,
One small suggestion, here we are implementing plane color management (pre blending).
Can we make the names of functions also to reflect the same. Later support for
Pipe level color management (post blending) can be added on top of it using the same
UAPI design.

As discussed already on the series sent by us, refer below:
https://patchwork.freedesktop.org/series/129811/

If you feel ok with the changes, please include support for hw_caps property, changes for
adding 1dlut and 3x3 CTM and the fixes called out in the same in your series. This will help in
having a unified discussion on the DRM core changes.

Thanks & Regards,
Uma Shankar

> There are plenty of things that I would like to see here but haven't had a chance
> to look at. These will (hopefully) be addressed in future iterations, either in VKMS
> or amdgpu:
>  - PQ transfer function
>  - Add custom LUT colorops to VKMS
>  - Add pre-blending 3DLUT
>  - How to support HW which can't bypass entire pipeline?
>  - Add ability to create colorops that don't have BYPASS
>  - Can we do a LOAD / COMMIT model for LUTs (and other properties)?
>  - read-only scaling colorop which defines scaling taps and position
>  - read-only color format colorop to define supported color formats
>    for a pipeline
>  - named matrices, for things like converting YUV to RGB
> 
> IGT tests can be found at
> https://gitlab.freedesktop.org/hwentland/igt-gpu-tools/-/merge_requests/1
> 
> IGT patches are also being sent to the igt-dev mailing list.
> 
> If you prefer a gitlab MR for review you can find it at
> https://gitlab.freedesktop.org/hwentland/linux/-/merge_requests/5
> 
> v3:
>  - Abandon IOCTLs and discover colorops as clients iterate the pipeline
>  - Remove need for libdrm
>  - Add color_pipeline client cap and make mutually exclusive with
>    COLOR_RANGE and COLOR_ENCODING properties
>  - add CTM colorop to VKMS
>  - Use include way for kunit testing static functions (Arthur)
>  - Make TYPE a range property
>  - Move enum drm_colorop_type to uapi header
>  - and a bunch of smaller bits that are highlighted in the relevant commit
>    description
> 
> v2:
>  - Rebased on drm-misc-next
>  - Introduce a VKMS Kunit so we can test LUT functionality in vkms_composer
>  - Incorporate feedback in color_pipeline.rst doc
>  - Add support for sRGB inverse EOTF
>  - Add 2nd enumerated TF colorop to VKMS
>  - Fix LUTs and some issues with applying LUTs in VKMS
> 
> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Pekka Paalanen <pekka.paalanen@xxxxxxxxxxxxx>
> Cc: Simon Ser <contact@xxxxxxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Cc: Melissa Wen <mwen@xxxxxxxxxx>
> Cc: Jonas Ådahl <jadahl@xxxxxxxxxx>
> Cc: Sebastian Wick <sebastian.wick@xxxxxxxxxx>
> Cc: Shashank Sharma <shashank.sharma@xxxxxxx>
> Cc: Alexander Goins <agoins@xxxxxxxxxx>
> Cc: Joshua Ashton <joshua@xxxxxxxxx>
> Cc: Michel Dänzer <mdaenzer@xxxxxxxxxx>
> Cc: Aleix Pol <aleixpol@xxxxxxx>
> Cc: Xaver Hugl <xaver.hugl@xxxxxxxxx>
> Cc: Victoria Brekenfeld <victoria@xxxxxxxxxxxx>
> Cc: Sima <daniel@xxxxxxxx>
> Cc: Uma Shankar <uma.shankar@xxxxxxxxx>
> Cc: Naseer Ahmed <quic_naseer@xxxxxxxxxxx>
> Cc: Christopher Braga <quic_cbraga@xxxxxxxxxxx>
> Cc: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> Cc: Arthur Grillo <arthurgrillo@xxxxxxxxxx>
> Cc: Hector Martin <marcan@xxxxxxxxx>
> Cc: Liviu Dudau <Liviu.Dudau@xxxxxxx>
> Cc: Sasha McIntosh <sashamcintosh@xxxxxxxxxx>
> 
> Harry Wentland (23):
>   drm: Don't treat 0 as -1 in drm_fixp2int_ceil
>   drm: Add helper for conversion from signed-magnitude
>   drm/vkms: Create separate Kconfig file for VKMS
>   drm/vkms: Add kunit tests for VKMS LUT handling
>   drm/vkms: Avoid reading beyond LUT array
>   drm/doc/rfc: Describe why prescriptive color pipeline is needed
>   drm/colorop: Introduce new drm_colorop mode object
>   drm/colorop: Add TYPE property
>   drm/color: Add 1D Curve subtype
>   drm/colorop: Add BYPASS property
>   drm/colorop: Add NEXT property
>   drm/colorop: Add atomic state print for drm_colorop
>   drm/plane: Add COLOR PIPELINE property
>   drm/colorop: Add NEXT to colorop state print
>   drm/vkms: Add enumerated 1D curve colorop
>   drm/vkms: Add kunit tests for linear and sRGB LUTs
>   drm/colorop: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
>   drm/colorop: Add 3x4 CTM type
>   drm/vkms: Pull apply_colorop out of pre_blend_color_transform
>   drm/vkms: Use s32 for internal color pipeline precision
>   drm/vkms: add 3x4 matrix in color pipeline
>   drm/tests: Add a few tests around drm_fixed.h
>   drm/vkms: Add tests for CTM handling
> 
>  Documentation/gpu/rfc/color_pipeline.rst      | 352 ++++++++
>  drivers/gpu/drm/Kconfig                       |  14 +-
>  drivers/gpu/drm/Makefile                      |   1 +
>  drivers/gpu/drm/drm_atomic.c                  | 146 ++++
>  drivers/gpu/drm/drm_atomic_helper.c           |  12 +
>  drivers/gpu/drm/drm_atomic_state_helper.c     |   5 +
>  drivers/gpu/drm/drm_atomic_uapi.c             | 161 ++++
>  drivers/gpu/drm/drm_colorop.c                 | 292 +++++++
>  drivers/gpu/drm/drm_ioctl.c                   |   7 +
>  drivers/gpu/drm/drm_mode_config.c             |   7 +
>  drivers/gpu/drm/drm_plane_helper.c            |   2 +-
>  drivers/gpu/drm/tests/Makefile                |   3 +-
>  drivers/gpu/drm/tests/drm_fixp_test.c         |  69 ++
>  drivers/gpu/drm/vkms/Kconfig                  |  20 +
>  drivers/gpu/drm/vkms/Makefile                 |   4 +-
>  drivers/gpu/drm/vkms/tests/.kunitconfig       |   4 +
>  drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 355 ++++++++
>  drivers/gpu/drm/vkms/vkms_colorop.c           | 115 +++
>  drivers/gpu/drm/vkms/vkms_composer.c          | 131 ++-
>  drivers/gpu/drm/vkms/vkms_drv.h               |   8 +
>  drivers/gpu/drm/vkms/vkms_luts.c              | 802 ++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_luts.h              |  12 +
>  drivers/gpu/drm/vkms/vkms_plane.c             |   2 +
>  include/drm/drm_atomic.h                      |  82 ++
>  include/drm/drm_atomic_uapi.h                 |   3 +
>  include/drm/drm_colorop.h                     | 258 ++++++
>  include/drm/drm_file.h                        |   7 +
>  include/drm/drm_fixed.h                       |  20 +-
>  include/drm/drm_mode_config.h                 |  18 +
>  include/drm/drm_plane.h                       |  10 +
>  include/uapi/drm/drm.h                        |  18 +
>  include/uapi/drm/drm_mode.h                   |  22 +
>  32 files changed, 2939 insertions(+), 23 deletions(-)  create mode 100644
> Documentation/gpu/rfc/color_pipeline.rst
>  create mode 100644 drivers/gpu/drm/drm_colorop.c  create mode 100644
> drivers/gpu/drm/tests/drm_fixp_test.c
>  create mode 100644 drivers/gpu/drm/vkms/Kconfig  create mode 100644
> drivers/gpu/drm/vkms/tests/.kunitconfig
>  create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.c  create mode 100644
> drivers/gpu/drm/vkms/vkms_luts.h  create mode 100644
> include/drm/drm_colorop.h
> 
> --
> 2.42.1





[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