Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On Tue, 7 Nov 2023 11:58:26 -0500 Harry Wentland wrote: > On 2023-11-07 04:55, Pekka Paalanen wrote: > > On Mon, 6 Nov 2023 11:19:27 -0500 > > Harry Wentland wrote: > > > >> On 2023-10-20 06:36, Pekka Paalanen wrote: > >>> On Thu, 19 Oct 2023 10:56:40 -0400 > >>> Harry Wentland wrote: > >>> > On 2023-10-10 12:13, Melissa Wen wrote: > > O 09/08, Harry Wentland wrote: > >> Signed-off-by: Harry Wentland > >>> > >>> ... > >>> > > Also, with this new plane API in place, I understand that we will > > already need think on how to deal with the mixing between old drm color > > properties (color encoding and color range) and these new way of setting > > plane color properties. IIUC, Pekka asked a related question about it > > when talking about CRTC automatic RGB->YUV (?) > > > > We'll still need to confirm whether we'll want to deprecate these > existing properties. If we do that we'd want a client prop. Things > should still work without deprecating them, if drivers just pick up > after the initial encoding and range CSC. > > But realistically it might be better to deprecate them and turn them > into explicit colorops. > >>> > >>> The existing properties would need to be explicitly reflected in the > >>> new pipelines anyway, otherwise there would always be doubt at which > >>> point of a pipeline the old properties apply, and they might even > >>> need to change positions between pipelines. > >>> > >>> I think it is simply easier to just hide all old color related > >>> properties when userspace sets the client-cap to enable pipelines. The > >>> problem is to make sure to hide all old properties on all drivers that > >>> support the client-cap. > >>> > >>> As a pipeline must be complete (describe everything that happens to > >>> pixel values), it's going to be a flag day per driver. > >>> > >>> Btw. the plane FB YUV->RGB conversion needs a colorop in every pipeline > >>> as well. Maybe it's purely informative and non-configurable, keyed by > >>> FB pixel format, but still. > >>> > >>> We also need a colorop to represent sample filtering, e.g. bilinear, > >>> like I think Sebastian may have mentioned in the past. Everything > >>> before the sample filter happens "per tap" as Joshua Ashton put it, and > >>> everything after it happens on the sample that was computed as a > >>> weighted average of the filter tap inputs (texels). > >>> > >>> There could be colorops other than sample filtering that operate on > >>> more than one sample at a time, like blur or sharpness. There could > >>> even be colorops that change the image size like adding padding that > >>> the following colorop hardware requires, and then yet another colorop > >>> that clips that padding away. For an example, see > >>> https://lists.freedesktop.org/archives/dri-devel/2023-October/427015.html > >>> > >>> If that padding and its color can affect the pipeline results of the > >>> pixels near the padding (e.g. some convolution is applied with them, > >>> which may be the reason why padding is necessary to begin with), then > >>> it would be best to model it. > >>> > >> > >> I hear you but I'm also somewhat shying away from defining this at this > >> point. > > > > Would you define them before the new UAPI is released though? > > > > I agree there is no need to have them in this patch series, but I think > > we'd hit the below problems if the UAPI is released without them. > > > >> There are already too many things that need to happen and I will focus on > >> the > >> actual color blocks (LUTs, matrices) first. We'll always be able to add a > >> new > >> (read-only) colorop type to define scaling and tap behavior at any point > >> and > >> a client is free to ignore a color pipeline if it doesn't find any > >> tap/scale > >> info in it. > > > > How would userspace know to look for tap/scale info, if there is no > > upstream definition even on paper? > > > > So far OSes did not care about this. Whether that's good or bad is > something everyone can answer for themselves. > > If you write a compositor and really need this you can just ignore > color pipelines that don't have this, i.e., you'll probably want > to wait with implementing color pipeline support until you have what > you need from DRM/KMS. > > This is not to say I don't want to have support for Weston. But I'm > wondering if we place too much importance on getting every little > thing figured out whereas we could be making forward progress and > address more aspects of a color pipeline in the future. There is a > reason gamescope has made a huge difference in driving the color > management work forward. > > > And the opposite case, if someone writes userspace without tap/scale > > colorops, and then drivers add those, and there is no pipeline without > > them, because they always exist. Would that userspace disregard all > > those pipelines be
Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On Wed, Nov 8, 2023 at 11:16 AM Pekka Paalanen wrote: > > On Tue, 7 Nov 2023 11:58:26 -0500 > Harry Wentland wrote: > > > On 2023-11-07 04:55, Pekka Paalanen wrote: > > > On Mon, 6 Nov 2023 11:19:27 -0500 > > > Harry Wentland wrote: > > > > > >> On 2023-10-20 06:36, Pekka Paalanen wrote: > > >>> On Thu, 19 Oct 2023 10:56:40 -0400 > > >>> Harry Wentland wrote: > > >>> > > On 2023-10-10 12:13, Melissa Wen wrote: > > > O 09/08, Harry Wentland wrote: > > >> Signed-off-by: Harry Wentland > > >>> > > >>> ... > > >>> > > > Also, with this new plane API in place, I understand that we will > > > already need think on how to deal with the mixing between old drm > > > color > > > properties (color encoding and color range) and these new way of > > > setting > > > plane color properties. IIUC, Pekka asked a related question about it > > > when talking about CRTC automatic RGB->YUV (?) > > > > > > > We'll still need to confirm whether we'll want to deprecate these > > existing properties. If we do that we'd want a client prop. Things > > should still work without deprecating them, if drivers just pick up > > after the initial encoding and range CSC. > > > > But realistically it might be better to deprecate them and turn them > > into explicit colorops. > > >>> > > >>> The existing properties would need to be explicitly reflected in the > > >>> new pipelines anyway, otherwise there would always be doubt at which > > >>> point of a pipeline the old properties apply, and they might even > > >>> need to change positions between pipelines. > > >>> > > >>> I think it is simply easier to just hide all old color related > > >>> properties when userspace sets the client-cap to enable pipelines. The > > >>> problem is to make sure to hide all old properties on all drivers that > > >>> support the client-cap. > > >>> > > >>> As a pipeline must be complete (describe everything that happens to > > >>> pixel values), it's going to be a flag day per driver. > > >>> > > >>> Btw. the plane FB YUV->RGB conversion needs a colorop in every pipeline > > >>> as well. Maybe it's purely informative and non-configurable, keyed by > > >>> FB pixel format, but still. > > >>> > > >>> We also need a colorop to represent sample filtering, e.g. bilinear, > > >>> like I think Sebastian may have mentioned in the past. Everything > > >>> before the sample filter happens "per tap" as Joshua Ashton put it, and > > >>> everything after it happens on the sample that was computed as a > > >>> weighted average of the filter tap inputs (texels). > > >>> > > >>> There could be colorops other than sample filtering that operate on > > >>> more than one sample at a time, like blur or sharpness. There could > > >>> even be colorops that change the image size like adding padding that > > >>> the following colorop hardware requires, and then yet another colorop > > >>> that clips that padding away. For an example, see > > >>> https://lists.freedesktop.org/archives/dri-devel/2023-October/427015.html > > >>> > > >>> If that padding and its color can affect the pipeline results of the > > >>> pixels near the padding (e.g. some convolution is applied with them, > > >>> which may be the reason why padding is necessary to begin with), then > > >>> it would be best to model it. > > >>> > > >> > > >> I hear you but I'm also somewhat shying away from defining this at this > > >> point. > > > > > > Would you define them before the new UAPI is released though? > > > > > > I agree there is no need to have them in this patch series, but I think > > > we'd hit the below problems if the UAPI is released without them. > > > > > >> There are already too many things that need to happen and I will focus > > >> on the > > >> actual color blocks (LUTs, matrices) first. We'll always be able to add > > >> a new > > >> (read-only) colorop type to define scaling and tap behavior at any point > > >> and > > >> a client is free to ignore a color pipeline if it doesn't find any > > >> tap/scale > > >> info in it. > > > > > > How would userspace know to look for tap/scale info, if there is no > > > upstream definition even on paper? > > > > > > > So far OSes did not care about this. Whether that's good or bad is > > something everyone can answer for themselves. > > > > If you write a compositor and really need this you can just ignore > > color pipelines that don't have this, i.e., you'll probably want > > to wait with implementing color pipeline support until you have what > > you need from DRM/KMS. > > > > This is not to say I don't want to have support for Weston. But I'm > > wondering if we place too much importance on getting every little > > thing figured out whereas we could be making forward progress and > > address more aspects of a color pipeline in the future. There is a > > reason gamescope has made a huge difference in driving the color > > management work forward. > > > > > And the opp
RE: [RFC PATCH v2 00/17] Color Pipeline API w/ VKMS
> -Original Message- > From: Harry Wentland > Sent: Friday, October 20, 2023 2:51 AM > To: dri-de...@lists.freedesktop.org > Cc: wayland-devel@lists.freedesktop.org; Harry Wentland > ; Ville Syrjala ; Pekka > Paalanen ; Simon Ser ; > Melissa Wen ; Jonas Ådahl ; Sebastian > Wick ; Shashank Sharma > ; Alexander Goins ; Joshua > Ashton ; Michel Dänzer ; Aleix Pol > ; Xaver Hugl ; Victoria Brekenfeld > ; Sima ; Shankar, Uma > ; Naseer Ahmed ; > Christopher Braga ; Abhinav Kumar > ; Arthur Grillo ; Hector > Martin ; Liviu Dudau ; Sasha > McIntosh > Subject: [RFC PATCH v2 00/17] Color Pipeline API w/ VKMS > > This is an early 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 we have an IGT > test that confirms that sRGB EOTF, followed by its inverse gives us expected > results within +/- 1 8 bpc codepoint value. > > 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-15: Add core DRM API bits > - Patches 15-17: VKMS implementation > > 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: > - Abandon IOCTLs and discover colorops as clients iterate the pipeline > - Add color_pipeline client cap and deprecate existing color encoding and >color range properties. >See https://lists.freedesktop.org/archives/dri-devel/2023- > September/422643.html > - Add CTM colorop to VKMS > - Add custom LUT colorops to VKMS > - Add pre-blending 3DLUT with tetrahedral interpolation to VKMS > - 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)? > > 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. > > libdrm changes to support the new IOCTLs are at > https://gitlab.freedesktop.org/hwentland/drm/-/merge_requests/1 > > If you prefer a gitlab MR for review you can find it at > https://gitlab.freedesktop.org/hwentland/linux/-/merge_requests/5 > > A slightly different approach for a Color Pipeline API was sent by Uma Shankar > and can be found at https://patchwork.freedesktop.org/series/123024/ > > The main difference is that his approach is not introducing a new DRM core > object > but instead exposes color pipelines via blob properties. > There are pros and cons to both approaches. Thanks Harry and all others who have actively contributed to the design and discussions thus far. Due to other commitments, we couldn't participate in XDC this time and also the delay on our part. Our apologies. We looked at the approach and are aligned to go with property-based design, with some suggestions. Will follow in comments in respective patches. We are also in process of trying this for Intel's hardware to identify if any gaps. Regards, Uma Shankar > 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 > Cc: Pekka Paalanen > Cc: Simon Ser > Cc: Harry Wentland > Cc: Melissa Wen > Cc: Jonas Ådahl > Cc: Sebastian Wick > Cc: Shashank Sharma > Cc: Alexander Goins > Cc: Joshua Ashton > Cc: Michel Dänzer > Cc: Aleix Pol > Cc: Xaver Hugl > Cc: Victoria Brekenfeld > Cc: Sima > Cc: Uma Shankar > Cc: Naseer Ahmed > Cc: Christopher Braga > Cc: Abhinav Kumar > Cc: Arthur Grillo > Cc: Hector Martin > Cc: Liviu Dudau > Cc: Sasha McIntosh > > Harry Wentland (17): > drm/atomic: Allow get_value for immutable properties on atomic drivers > drm: Don't treat 0 as -1 in drm_fixp2int_ceil > 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/colorop: Add new IOCTLs to retrieve drm_colorop objects > drm/plane: Add COLOR PIPELINE property > drm/colorop: Add NEXT to colorop state print > drm/vkms: Add enumerated 1D curve colorop > drm
RE: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed
> -Original Message- > From: Harry Wentland > Sent: Friday, October 20, 2023 2:51 AM > To: dri-de...@lists.freedesktop.org > Cc: wayland-devel@lists.freedesktop.org; Harry Wentland > ; Ville Syrjala ; Pekka > Paalanen ; Simon Ser ; > Melissa Wen ; Jonas Ådahl ; Sebastian > Wick ; Shashank Sharma > ; Alexander Goins ; Joshua > Ashton ; Michel Dänzer ; Aleix Pol > ; Xaver Hugl ; Victoria Brekenfeld > ; Sima ; Shankar, Uma > ; Naseer Ahmed ; > Christopher Braga ; Abhinav Kumar > ; Arthur Grillo ; Hector > Martin ; Liviu Dudau ; Sasha > McIntosh > Subject: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color > pipeline is needed > > v2: > - Update colorop visualizations to match reality (Sebastian, Alex Hung) > - Updated wording (Pekka) > - Change BYPASS wording to make it non-mandatory (Sebastian) > - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane Property >section (Pekka) > - Use PQ EOTF instead of its inverse in Pipeline Programming example > (Melissa) > - Add "Driver Implementer's Guide" section (Pekka) > - Add "Driver Forward/Backward Compatibility" section (Sebastian, Pekka) > > Signed-off-by: Harry Wentland > Cc: Ville Syrjala > Cc: Pekka Paalanen > Cc: Simon Ser > Cc: Harry Wentland > Cc: Melissa Wen > Cc: Jonas Ådahl > Cc: Sebastian Wick > Cc: Shashank Sharma > Cc: Alexander Goins > Cc: Joshua Ashton > Cc: Michel Dänzer > Cc: Aleix Pol > Cc: Xaver Hugl > Cc: Victoria Brekenfeld > Cc: Sima > Cc: Uma Shankar > Cc: Naseer Ahmed > Cc: Christopher Braga > Cc: Abhinav Kumar > Cc: Arthur Grillo > Cc: Hector Martin > Cc: Liviu Dudau > Cc: Sasha McIntosh > --- > Documentation/gpu/rfc/color_pipeline.rst | 347 +++ > 1 file changed, 347 insertions(+) > create mode 100644 Documentation/gpu/rfc/color_pipeline.rst > > diff --git a/Documentation/gpu/rfc/color_pipeline.rst > b/Documentation/gpu/rfc/color_pipeline.rst > new file mode 100644 > index ..af5f2ea29116 > --- /dev/null > +++ b/Documentation/gpu/rfc/color_pipeline.rst > @@ -0,0 +1,347 @@ > + > +Linux Color Pipeline API > + > + > +What problem are we solving? > + > + > +We would like to support pre-, and post-blending complex color > +transformations in display controller hardware in order to allow for > +HW-supported HDR use-cases, as well as to provide support to > +color-managed applications, such as video or image editors. > + > +It is possible to support an HDR output on HW supporting the Colorspace > +and HDR Metadata drm_connector properties, but that requires the > +compositor or application to render and compose the content into one > +final buffer intended for display. Doing so is costly. > + > +Most modern display HW offers various 1D LUTs, 3D LUTs, matrices, and > +other operations to support color transformations. These operations are > +often implemented in fixed-function HW and therefore much more power > +efficient than performing similar operations via shaders or CPU. > + > +We would like to make use of this HW functionality to support complex > +color transformations with no, or minimal CPU or shader load. > + > + > +How are other OSes solving this problem? > + > + > +The most widely supported use-cases regard HDR content, whether video > +or gaming. > + > +Most OSes will specify the source content format (color gamut, encoding > +transfer function, and other metadata, such as max and average light levels) > to a > driver. > +Drivers will then program their fixed-function HW accordingly to map > +from a source content buffer's space to a display's space. > + > +When fixed-function HW is not available the compositor will assemble a > +shader to ask the GPU to perform the transformation from the source > +content format to the display's format. > + > +A compositor's mapping function and a driver's mapping function are > +usually entirely separate concepts. On OSes where a HW vendor has no > +insight into closed-source compositor code such a vendor will tune > +their color management code to visually match the compositor's. On > +other OSes, where both mapping functions are open to an implementer they will > ensure both mappings match. > + > +This results in mapping algorithm lock-in, meaning that no-one alone > +can experiment with or introduce new mapping algorithms and achieve > +consistent results regardless of which implementation path is taken. > + > +Why is Linux different? > +=== > + > +Unlike other OSes, where there is one compositor for one or more > +drivers, on Linux we have a many-to-many relationship. Many compositors; > many drivers. > +In addition each compositor vendor or community has their own view of > +how color management should be done. This is what makes Linux so beautiful. > + > +This means that a HW vendor can now no longer tune their driver to one > +compositor, as tuning it t
Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On 11/8/23 12:18, Shankar, Uma wrote: -Original Message- From: Harry Wentland Sent: Friday, October 20, 2023 2:51 AM To: dri-de...@lists.freedesktop.org Cc: wayland-devel@lists.freedesktop.org; Harry Wentland ; Ville Syrjala ; Pekka Paalanen ; Simon Ser ; Melissa Wen ; Jonas Ådahl ; Sebastian Wick ; Shashank Sharma ; Alexander Goins ; Joshua Ashton ; Michel Dänzer ; Aleix Pol ; Xaver Hugl ; Victoria Brekenfeld ; Sima ; Shankar, Uma ; Naseer Ahmed ; Christopher Braga ; Abhinav Kumar ; Arthur Grillo ; Hector Martin ; Liviu Dudau ; Sasha McIntosh Subject: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed v2: - Update colorop visualizations to match reality (Sebastian, Alex Hung) - Updated wording (Pekka) - Change BYPASS wording to make it non-mandatory (Sebastian) - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane Property section (Pekka) - Use PQ EOTF instead of its inverse in Pipeline Programming example (Melissa) - Add "Driver Implementer's Guide" section (Pekka) - Add "Driver Forward/Backward Compatibility" section (Sebastian, Pekka) Signed-off-by: Harry Wentland Cc: Ville Syrjala Cc: Pekka Paalanen Cc: Simon Ser Cc: Harry Wentland Cc: Melissa Wen Cc: Jonas Ådahl Cc: Sebastian Wick Cc: Shashank Sharma Cc: Alexander Goins Cc: Joshua Ashton Cc: Michel Dänzer Cc: Aleix Pol Cc: Xaver Hugl Cc: Victoria Brekenfeld Cc: Sima Cc: Uma Shankar Cc: Naseer Ahmed Cc: Christopher Braga Cc: Abhinav Kumar Cc: Arthur Grillo Cc: Hector Martin Cc: Liviu Dudau Cc: Sasha McIntosh --- Documentation/gpu/rfc/color_pipeline.rst | 347 +++ 1 file changed, 347 insertions(+) create mode 100644 Documentation/gpu/rfc/color_pipeline.rst diff --git a/Documentation/gpu/rfc/color_pipeline.rst b/Documentation/gpu/rfc/color_pipeline.rst new file mode 100644 index ..af5f2ea29116 --- /dev/null +++ b/Documentation/gpu/rfc/color_pipeline.rst @@ -0,0 +1,347 @@ + +Linux Color Pipeline API + + +What problem are we solving? + + +We would like to support pre-, and post-blending complex color +transformations in display controller hardware in order to allow for +HW-supported HDR use-cases, as well as to provide support to +color-managed applications, such as video or image editors. + +It is possible to support an HDR output on HW supporting the Colorspace +and HDR Metadata drm_connector properties, but that requires the +compositor or application to render and compose the content into one +final buffer intended for display. Doing so is costly. + +Most modern display HW offers various 1D LUTs, 3D LUTs, matrices, and +other operations to support color transformations. These operations are +often implemented in fixed-function HW and therefore much more power +efficient than performing similar operations via shaders or CPU. + +We would like to make use of this HW functionality to support complex +color transformations with no, or minimal CPU or shader load. + + +How are other OSes solving this problem? + + +The most widely supported use-cases regard HDR content, whether video +or gaming. + +Most OSes will specify the source content format (color gamut, encoding +transfer function, and other metadata, such as max and average light levels) to a driver. +Drivers will then program their fixed-function HW accordingly to map +from a source content buffer's space to a display's space. + +When fixed-function HW is not available the compositor will assemble a +shader to ask the GPU to perform the transformation from the source +content format to the display's format. + +A compositor's mapping function and a driver's mapping function are +usually entirely separate concepts. On OSes where a HW vendor has no +insight into closed-source compositor code such a vendor will tune +their color management code to visually match the compositor's. On +other OSes, where both mapping functions are open to an implementer they will ensure both mappings match. + +This results in mapping algorithm lock-in, meaning that no-one alone +can experiment with or introduce new mapping algorithms and achieve +consistent results regardless of which implementation path is taken. + +Why is Linux different? +=== + +Unlike other OSes, where there is one compositor for one or more +drivers, on Linux we have a many-to-many relationship. Many compositors; many drivers. +In addition each compositor vendor or community has their own view of +how color management should be done. This is what makes Linux so beautiful. + +This means that a HW vendor can now no longer tune their driver to one +compositor, as tuning it to one could make it look fairly different +from another compositor's color mapping. + +We need a better solution. + + +Descriptive API +=== + +An API that describes the source and destination colorspa
Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On 2023-11-08 06:40, Sebastian Wick wrote: > On Wed, Nov 8, 2023 at 11:16 AM Pekka Paalanen wrote: >> >> On Tue, 7 Nov 2023 11:58:26 -0500 >> Harry Wentland wrote: >> >>> On 2023-11-07 04:55, Pekka Paalanen wrote: On Mon, 6 Nov 2023 11:19:27 -0500 Harry Wentland wrote: > On 2023-10-20 06:36, Pekka Paalanen wrote: >> On Thu, 19 Oct 2023 10:56:40 -0400 >> Harry Wentland wrote: >> >>> On 2023-10-10 12:13, Melissa Wen wrote: O 09/08, Harry Wentland wrote: > Signed-off-by: Harry Wentland >> >> ... >> Also, with this new plane API in place, I understand that we will already need think on how to deal with the mixing between old drm color properties (color encoding and color range) and these new way of setting plane color properties. IIUC, Pekka asked a related question about it when talking about CRTC automatic RGB->YUV (?) >>> >>> We'll still need to confirm whether we'll want to deprecate these >>> existing properties. If we do that we'd want a client prop. Things >>> should still work without deprecating them, if drivers just pick up >>> after the initial encoding and range CSC. >>> >>> But realistically it might be better to deprecate them and turn them >>> into explicit colorops. >> >> The existing properties would need to be explicitly reflected in the >> new pipelines anyway, otherwise there would always be doubt at which >> point of a pipeline the old properties apply, and they might even >> need to change positions between pipelines. >> >> I think it is simply easier to just hide all old color related >> properties when userspace sets the client-cap to enable pipelines. The >> problem is to make sure to hide all old properties on all drivers that >> support the client-cap. >> >> As a pipeline must be complete (describe everything that happens to >> pixel values), it's going to be a flag day per driver. >> >> Btw. the plane FB YUV->RGB conversion needs a colorop in every pipeline >> as well. Maybe it's purely informative and non-configurable, keyed by >> FB pixel format, but still. >> >> We also need a colorop to represent sample filtering, e.g. bilinear, >> like I think Sebastian may have mentioned in the past. Everything >> before the sample filter happens "per tap" as Joshua Ashton put it, and >> everything after it happens on the sample that was computed as a >> weighted average of the filter tap inputs (texels). >> >> There could be colorops other than sample filtering that operate on >> more than one sample at a time, like blur or sharpness. There could >> even be colorops that change the image size like adding padding that >> the following colorop hardware requires, and then yet another colorop >> that clips that padding away. For an example, see >> https://lists.freedesktop.org/archives/dri-devel/2023-October/427015.html >> >> If that padding and its color can affect the pipeline results of the >> pixels near the padding (e.g. some convolution is applied with them, >> which may be the reason why padding is necessary to begin with), then >> it would be best to model it. >> > > I hear you but I'm also somewhat shying away from defining this at this > point. Would you define them before the new UAPI is released though? I agree there is no need to have them in this patch series, but I think we'd hit the below problems if the UAPI is released without them. > There are already too many things that need to happen and I will focus on > the > actual color blocks (LUTs, matrices) first. We'll always be able to add a > new > (read-only) colorop type to define scaling and tap behavior at any point > and > a client is free to ignore a color pipeline if it doesn't find any > tap/scale > info in it. How would userspace know to look for tap/scale info, if there is no upstream definition even on paper? >>> >>> So far OSes did not care about this. Whether that's good or bad is >>> something everyone can answer for themselves. >>> >>> If you write a compositor and really need this you can just ignore >>> color pipelines that don't have this, i.e., you'll probably want >>> to wait with implementing color pipeline support until you have what >>> you need from DRM/KMS. >>> >>> This is not to say I don't want to have support for Weston. But I'm >>> wondering if we place too much importance on getting every little >>> thing figured out whereas we could be making forward progress and >>> address more aspects of a color pipeline in the future. There is a >>> reason gamescope has made a huge difference in driving the color >>> management work forward. >>> And the opposite case, if someone writes userspace withou
Re: [RFC PATCH v2 00/17] Color Pipeline API w/ VKMS
On 2023-11-08 06:54, Shankar, Uma wrote: > > >> -Original Message- >> From: Harry Wentland >> Sent: Friday, October 20, 2023 2:51 AM >> To: dri-de...@lists.freedesktop.org >> Cc: wayland-devel@lists.freedesktop.org; Harry Wentland >> ; Ville Syrjala ; >> Pekka >> Paalanen ; Simon Ser ; >> Melissa Wen ; Jonas Ådahl ; Sebastian >> Wick ; Shashank Sharma >> ; Alexander Goins ; Joshua >> Ashton ; Michel Dänzer ; Aleix Pol >> ; Xaver Hugl ; Victoria Brekenfeld >> ; Sima ; Shankar, Uma >> ; Naseer Ahmed ; >> Christopher Braga ; Abhinav Kumar >> ; Arthur Grillo ; Hector >> Martin ; Liviu Dudau ; Sasha >> McIntosh >> Subject: [RFC PATCH v2 00/17] Color Pipeline API w/ VKMS >> >> This is an early 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 we have an IGT >> test that confirms that sRGB EOTF, followed by its inverse gives us expected >> results within +/- 1 8 bpc codepoint value. >> >> 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-15: Add core DRM API bits >> - Patches 15-17: VKMS implementation >> >> 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: >> - Abandon IOCTLs and discover colorops as clients iterate the pipeline >> - Add color_pipeline client cap and deprecate existing color encoding and >>color range properties. >>See https://lists.freedesktop.org/archives/dri-devel/2023- >> September/422643.html >> - Add CTM colorop to VKMS >> - Add custom LUT colorops to VKMS >> - Add pre-blending 3DLUT with tetrahedral interpolation to VKMS >> - 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)? >> >> 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. >> >> libdrm changes to support the new IOCTLs are at >> https://gitlab.freedesktop.org/hwentland/drm/-/merge_requests/1 >> >> If you prefer a gitlab MR for review you can find it at >> https://gitlab.freedesktop.org/hwentland/linux/-/merge_requests/5 >> >> A slightly different approach for a Color Pipeline API was sent by Uma >> Shankar >> and can be found at https://patchwork.freedesktop.org/series/123024/ >> >> The main difference is that his approach is not introducing a new DRM core >> object >> but instead exposes color pipelines via blob properties. >> There are pros and cons to both approaches. > > Thanks Harry and all others who have actively contributed to the design and > discussions thus far. > > Due to other commitments, we couldn't participate in XDC this time and also > the delay on our part. Our apologies. > > We looked at the approach and are aligned to go with property-based design, > with some suggestions. Will follow in comments in respective patches. > We are also in process of trying this for Intel's hardware to identify if any > gaps. > That's great to hear. Thanks, Uma. Harry > Regards, > Uma Shankar > >> 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 >> Cc: Pekka Paalanen >> Cc: Simon Ser >> Cc: Harry Wentland >> Cc: Melissa Wen >> Cc: Jonas Ådahl >> Cc: Sebastian Wick >> Cc: Shashank Sharma >> Cc: Alexander Goins >> Cc: Joshua Ashton >> Cc: Michel Dänzer >> Cc: Aleix Pol >> Cc: Xaver Hugl >> Cc: Victoria Brekenfeld >> Cc: Sima >> Cc: Uma Shankar >> Cc: Naseer Ahmed >> Cc: Christopher Braga >> Cc: Abhinav Kumar >> Cc: Arthur Grillo >> Cc: Hector Martin >> Cc: Liviu Dudau >> Cc: Sasha McIntosh >> >> Harry Wentland (17): >> drm/atomic: Allow get_value for immutable properties on atomic drivers >> drm: Don't treat 0 as -1 in drm_fixp2int_ceil >> 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 stat
Re: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On 2023-11-08 07:18, Shankar, Uma wrote: > > >> -Original Message- >> From: Harry Wentland >> Sent: Friday, October 20, 2023 2:51 AM >> To: dri-de...@lists.freedesktop.org >> Cc: wayland-devel@lists.freedesktop.org; Harry Wentland >> ; Ville Syrjala ; >> Pekka >> Paalanen ; Simon Ser ; >> Melissa Wen ; Jonas Ådahl ; Sebastian >> Wick ; Shashank Sharma >> ; Alexander Goins ; Joshua >> Ashton ; Michel Dänzer ; Aleix Pol >> ; Xaver Hugl ; Victoria Brekenfeld >> ; Sima ; Shankar, Uma >> ; Naseer Ahmed ; >> Christopher Braga ; Abhinav Kumar >> ; Arthur Grillo ; Hector >> Martin ; Liviu Dudau ; Sasha >> McIntosh >> Subject: [RFC PATCH v2 06/17] drm/doc/rfc: Describe why prescriptive color >> pipeline is needed >> >> v2: >> - Update colorop visualizations to match reality (Sebastian, Alex Hung) >> - Updated wording (Pekka) >> - Change BYPASS wording to make it non-mandatory (Sebastian) >> - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane Property >>section (Pekka) >> - Use PQ EOTF instead of its inverse in Pipeline Programming example >> (Melissa) >> - Add "Driver Implementer's Guide" section (Pekka) >> - Add "Driver Forward/Backward Compatibility" section (Sebastian, Pekka) >> >> Signed-off-by: Harry Wentland >> Cc: Ville Syrjala >> Cc: Pekka Paalanen >> Cc: Simon Ser >> Cc: Harry Wentland >> Cc: Melissa Wen >> Cc: Jonas Ådahl >> Cc: Sebastian Wick >> Cc: Shashank Sharma >> Cc: Alexander Goins >> Cc: Joshua Ashton >> Cc: Michel Dänzer >> Cc: Aleix Pol >> Cc: Xaver Hugl >> Cc: Victoria Brekenfeld >> Cc: Sima >> Cc: Uma Shankar >> Cc: Naseer Ahmed >> Cc: Christopher Braga >> Cc: Abhinav Kumar >> Cc: Arthur Grillo >> Cc: Hector Martin >> Cc: Liviu Dudau >> Cc: Sasha McIntosh >> --- >> Documentation/gpu/rfc/color_pipeline.rst | 347 +++ >> 1 file changed, 347 insertions(+) >> create mode 100644 Documentation/gpu/rfc/color_pipeline.rst >> >> diff --git a/Documentation/gpu/rfc/color_pipeline.rst >> b/Documentation/gpu/rfc/color_pipeline.rst >> new file mode 100644 >> index ..af5f2ea29116 >> --- /dev/null >> +++ b/Documentation/gpu/rfc/color_pipeline.rst >> @@ -0,0 +1,347 @@ >> + >> +Linux Color Pipeline API >> + >> + >> +What problem are we solving? >> + >> + >> +We would like to support pre-, and post-blending complex color >> +transformations in display controller hardware in order to allow for >> +HW-supported HDR use-cases, as well as to provide support to >> +color-managed applications, such as video or image editors. >> + >> +It is possible to support an HDR output on HW supporting the Colorspace >> +and HDR Metadata drm_connector properties, but that requires the >> +compositor or application to render and compose the content into one >> +final buffer intended for display. Doing so is costly. >> + >> +Most modern display HW offers various 1D LUTs, 3D LUTs, matrices, and >> +other operations to support color transformations. These operations are >> +often implemented in fixed-function HW and therefore much more power >> +efficient than performing similar operations via shaders or CPU. >> + >> +We would like to make use of this HW functionality to support complex >> +color transformations with no, or minimal CPU or shader load. >> + >> + >> +How are other OSes solving this problem? >> + >> + >> +The most widely supported use-cases regard HDR content, whether video >> +or gaming. >> + >> +Most OSes will specify the source content format (color gamut, encoding >> +transfer function, and other metadata, such as max and average light >> levels) to a >> driver. >> +Drivers will then program their fixed-function HW accordingly to map >> +from a source content buffer's space to a display's space. >> + >> +When fixed-function HW is not available the compositor will assemble a >> +shader to ask the GPU to perform the transformation from the source >> +content format to the display's format. >> + >> +A compositor's mapping function and a driver's mapping function are >> +usually entirely separate concepts. On OSes where a HW vendor has no >> +insight into closed-source compositor code such a vendor will tune >> +their color management code to visually match the compositor's. On >> +other OSes, where both mapping functions are open to an implementer they >> will >> ensure both mappings match. >> + >> +This results in mapping algorithm lock-in, meaning that no-one alone >> +can experiment with or introduce new mapping algorithms and achieve >> +consistent results regardless of which implementation path is taken. >> + >> +Why is Linux different? >> +=== >> + >> +Unlike other OSes, where there is one compositor for one or more >> +drivers, on Linux we have a many-to-many relationship. Many compositors; >> many drivers. >> +In addition each compositor vendor or community has their own view of >> +
Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On Wed, 8 Nov 2023 09:31:17 -0500 Harry Wentland wrote: > On 2023-11-08 06:40, Sebastian Wick wrote: > > On Wed, Nov 8, 2023 at 11:16 AM Pekka Paalanen wrote: > > > >> > >> On Tue, 7 Nov 2023 11:58:26 -0500 > >> Harry Wentland wrote: > >> > >>> On 2023-11-07 04:55, Pekka Paalanen wrote: > On Mon, 6 Nov 2023 11:19:27 -0500 > Harry Wentland wrote: > > > On 2023-10-20 06:36, Pekka Paalanen wrote: > >> On Thu, 19 Oct 2023 10:56:40 -0400 > >> Harry Wentland wrote: > >> > >>> On 2023-10-10 12:13, Melissa Wen wrote: > O 09/08, Harry Wentland wrote: > > Signed-off-by: Harry Wentland > >> > >> ... > >> > Also, with this new plane API in place, I understand that we will > already need think on how to deal with the mixing between old drm > color > properties (color encoding and color range) and these new way of > setting > plane color properties. IIUC, Pekka asked a related question about it > when talking about CRTC automatic RGB->YUV (?) > > >>> > >>> We'll still need to confirm whether we'll want to deprecate these > >>> existing properties. If we do that we'd want a client prop. Things > >>> should still work without deprecating them, if drivers just pick up > >>> after the initial encoding and range CSC. > >>> > >>> But realistically it might be better to deprecate them and turn them > >>> into explicit colorops. > >> > >> The existing properties would need to be explicitly reflected in the > >> new pipelines anyway, otherwise there would always be doubt at which > >> point of a pipeline the old properties apply, and they might even > >> need to change positions between pipelines. > >> > >> I think it is simply easier to just hide all old color related > >> properties when userspace sets the client-cap to enable pipelines. The > >> problem is to make sure to hide all old properties on all drivers that > >> support the client-cap. > >> > >> As a pipeline must be complete (describe everything that happens to > >> pixel values), it's going to be a flag day per driver. > >> > >> Btw. the plane FB YUV->RGB conversion needs a colorop in every pipeline > >> as well. Maybe it's purely informative and non-configurable, keyed by > >> FB pixel format, but still. > >> > >> We also need a colorop to represent sample filtering, e.g. bilinear, > >> like I think Sebastian may have mentioned in the past. Everything > >> before the sample filter happens "per tap" as Joshua Ashton put it, and > >> everything after it happens on the sample that was computed as a > >> weighted average of the filter tap inputs (texels). > >> > >> There could be colorops other than sample filtering that operate on > >> more than one sample at a time, like blur or sharpness. There could > >> even be colorops that change the image size like adding padding that > >> the following colorop hardware requires, and then yet another colorop > >> that clips that padding away. For an example, see > >> https://lists.freedesktop.org/archives/dri-devel/2023-October/427015.html > >> > >> If that padding and its color can affect the pipeline results of the > >> pixels near the padding (e.g. some convolution is applied with them, > >> which may be the reason why padding is necessary to begin with), then > >> it would be best to model it. > >> > > > > I hear you but I'm also somewhat shying away from defining this at this > > point. > > Would you define them before the new UAPI is released though? > > I agree there is no need to have them in this patch series, but I think > we'd hit the below problems if the UAPI is released without them. > > > There are already too many things that need to happen and I will focus > > on the > > actual color blocks (LUTs, matrices) first. We'll always be able to add > > a new > > (read-only) colorop type to define scaling and tap behavior at any > > point and > > a client is free to ignore a color pipeline if it doesn't find any > > tap/scale > > info in it. > > How would userspace know to look for tap/scale info, if there is no > upstream definition even on paper? > > >>> > >>> So far OSes did not care about this. Whether that's good or bad is > >>> something everyone can answer for themselves. > >>> > >>> If you write a compositor and really need this you can just ignore > >>> color pipelines that don't have this, i.e., you'll probably want > >>> to wait with implementing color pipeline support until you have what > >>> you need from DRM/KMS. > >>> > >>> This is not to say I don't want to have support for Weston. But I'm > >>> wondering if we place too much importance on getting ever
Re: [RFC PATCH 01/10] drm/doc/rfc: Describe why prescriptive color pipeline is needed
On 2023-11-08 11:19, Pekka Paalanen wrote: > On Wed, 8 Nov 2023 09:31:17 -0500 > Harry Wentland wrote: > >> On 2023-11-08 06:40, Sebastian Wick wrote: >>> On Wed, Nov 8, 2023 at 11:16 AM Pekka Paalanen wrote: >>> On Tue, 7 Nov 2023 11:58:26 -0500 Harry Wentland wrote: > On 2023-11-07 04:55, Pekka Paalanen wrote: >> On Mon, 6 Nov 2023 11:19:27 -0500 >> Harry Wentland wrote: >> >>> On 2023-10-20 06:36, Pekka Paalanen wrote: On Thu, 19 Oct 2023 10:56:40 -0400 Harry Wentland wrote: > On 2023-10-10 12:13, Melissa Wen wrote: >> O 09/08, Harry Wentland wrote: >>> Signed-off-by: Harry Wentland ... >> Also, with this new plane API in place, I understand that we will >> already need think on how to deal with the mixing between old drm >> color >> properties (color encoding and color range) and these new way of >> setting >> plane color properties. IIUC, Pekka asked a related question about it >> when talking about CRTC automatic RGB->YUV (?) >> > > We'll still need to confirm whether we'll want to deprecate these > existing properties. If we do that we'd want a client prop. Things > should still work without deprecating them, if drivers just pick up > after the initial encoding and range CSC. > > But realistically it might be better to deprecate them and turn them > into explicit colorops. The existing properties would need to be explicitly reflected in the new pipelines anyway, otherwise there would always be doubt at which point of a pipeline the old properties apply, and they might even need to change positions between pipelines. I think it is simply easier to just hide all old color related properties when userspace sets the client-cap to enable pipelines. The problem is to make sure to hide all old properties on all drivers that support the client-cap. As a pipeline must be complete (describe everything that happens to pixel values), it's going to be a flag day per driver. Btw. the plane FB YUV->RGB conversion needs a colorop in every pipeline as well. Maybe it's purely informative and non-configurable, keyed by FB pixel format, but still. We also need a colorop to represent sample filtering, e.g. bilinear, like I think Sebastian may have mentioned in the past. Everything before the sample filter happens "per tap" as Joshua Ashton put it, and everything after it happens on the sample that was computed as a weighted average of the filter tap inputs (texels). There could be colorops other than sample filtering that operate on more than one sample at a time, like blur or sharpness. There could even be colorops that change the image size like adding padding that the following colorop hardware requires, and then yet another colorop that clips that padding away. For an example, see https://lists.freedesktop.org/archives/dri-devel/2023-October/427015.html If that padding and its color can affect the pipeline results of the pixels near the padding (e.g. some convolution is applied with them, which may be the reason why padding is necessary to begin with), then it would be best to model it. >>> >>> I hear you but I'm also somewhat shying away from defining this at this >>> point. >> >> Would you define them before the new UAPI is released though? >> >> I agree there is no need to have them in this patch series, but I think >> we'd hit the below problems if the UAPI is released without them. >> >>> There are already too many things that need to happen and I will focus >>> on the >>> actual color blocks (LUTs, matrices) first. We'll always be able to add >>> a new >>> (read-only) colorop type to define scaling and tap behavior at any >>> point and >>> a client is free to ignore a color pipeline if it doesn't find any >>> tap/scale >>> info in it. >> >> How would userspace know to look for tap/scale info, if there is no >> upstream definition even on paper? >> > > So far OSes did not care about this. Whether that's good or bad is > something everyone can answer for themselves. > > If you write a compositor and really need this you can just ignore > color pipelines that don't have this, i.e., you'll probably want > to wait with implementing color pipeline support until you have what > you need from DRM/KMS. > > This is not to say I don't want to have support for Weston. But I'm > wond
[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. 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 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 Cc: Pekka Paalanen Cc: Simon Ser Cc: Harry Wentland Cc: Melissa Wen Cc: Jonas Ådahl Cc: Sebastian Wick Cc: Shashank Sharma Cc: Alexander Goins Cc: Joshua Ashton Cc: Michel Dänzer Cc: Aleix Pol Cc: Xaver Hugl Cc: Victoria Brekenfeld Cc: Sima Cc: Uma Shankar Cc: Naseer Ahmed Cc: Christopher Braga Cc: Abhinav Kumar Cc: Arthur Grillo Cc: Hector Martin Cc: Liviu Dudau Cc: Sasha McIntosh 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 | 35
[RFC PATCH v3 05/23] drm/vkms: Avoid reading beyond LUT array
When the floor LUT index (drm_fixp2int(lut_index) is the last
index of the array the ceil LUT index will point to an entry
beyond the array. Make sure we guard against it and use the
value of the floor LUT index.
v3:
- Drop bits from commit description that didn't contribute
anything of value
Signed-off-by: Harry Wentland
Cc: Arthur Grillo
---
drivers/gpu/drm/vkms/vkms_composer.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 6f942896036e..25b6b73bece8 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -123,6 +123,8 @@ static u16 apply_lut_to_channel_value(const struct
vkms_color_lut *lut, u16 chan
enum lut_channel channel)
{
s64 lut_index = get_lut_index(lut, channel_value);
+ u16 *floor_lut_value, *ceil_lut_value;
+ u16 floor_channel_value, ceil_channel_value;
/*
* This checks if `struct drm_color_lut` has any gap added by the
compiler
@@ -130,11 +132,15 @@ static u16 apply_lut_to_channel_value(const struct
vkms_color_lut *lut, u16 chan
*/
static_assert(sizeof(struct drm_color_lut) == sizeof(__u16) * 4);
- u16 *floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
- u16 *ceil_lut_value = (__u16 *)&lut->base[drm_fixp2int_ceil(lut_index)];
+ floor_lut_value = (__u16 *)&lut->base[drm_fixp2int(lut_index)];
+ if (drm_fixp2int(lut_index) == (lut->lut_length - 1))
+ /* We're at the end of the LUT array, use same value for ceil
and floor */
+ ceil_lut_value = floor_lut_value;
+ else
+ ceil_lut_value = (__u16
*)&lut->base[drm_fixp2int_ceil(lut_index)];
- u16 floor_channel_value = floor_lut_value[channel];
- u16 ceil_channel_value = ceil_lut_value[channel];
+ floor_channel_value = floor_lut_value[channel];
+ ceil_channel_value = ceil_lut_value[channel];
return lerp_u16(floor_channel_value, ceil_channel_value,
lut_index & DRM_FIXED_DECIMAL_MASK);
--
2.42.1
[RFC PATCH v3 01/23] drm: Don't treat 0 as -1 in drm_fixp2int_ceil
Unit testing this in VKMS shows that passing 0 into
this function returns -1, which is highly counter-
intuitive. Fix it by checking whether the input is
>= 0 instead of > 0.
Signed-off-by: Harry Wentland
Reviewed-by: Simon Ser
---
include/drm/drm_fixed.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_fixed.h b/include/drm/drm_fixed.h
index 6ea339d5de08..0c9f917a4d4b 100644
--- a/include/drm/drm_fixed.h
+++ b/include/drm/drm_fixed.h
@@ -95,7 +95,7 @@ static inline int drm_fixp2int_round(s64 a)
static inline int drm_fixp2int_ceil(s64 a)
{
- if (a > 0)
+ if (a >= 0)
return drm_fixp2int(a + DRM_FIXED_ALMOST_ONE);
else
return drm_fixp2int(a - DRM_FIXED_ALMOST_ONE);
--
2.42.1
[RFC PATCH v3 10/23] drm/colorop: Add BYPASS property
We want to be able to bypass each colorop at all times.
Introduce a new BYPASS boolean property for this.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_atomic_uapi.c | 6 +-
drivers/gpu/drm/drm_colorop.c | 15 +++
include/drm/drm_colorop.h | 20
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 52b9b48e5757..a8f7a8a6639a 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -670,7 +670,9 @@ static int drm_atomic_colorop_set_property(struct
drm_colorop *colorop,
struct drm_colorop_state *state, struct drm_file *file_priv,
struct drm_property *property, uint64_t val)
{
- if (property == colorop->curve_1d_type_property) {
+ if (property == colorop->bypass_property) {
+ state->bypass = val;
+ } else if (property == colorop->curve_1d_type_property) {
state->curve_1d_type = val;
} else {
drm_dbg_atomic(colorop->dev,
@@ -690,6 +692,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
{
if (property == colorop->type_property) {
*val = colorop->type;
+ } else if (property == colorop->bypass_property) {
+ *val = state->bypass;
} else if (property == colorop->curve_1d_type_property) {
*val = state->curve_1d_type;
} else {
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index b1c271f90a16..17ba11ae205b 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -78,6 +78,18 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->type_property,
colorop->type);
+ /* bypass */
+ /* TODO can we reuse the mode_config->active_prop? */
+ prop = drm_property_create_bool(dev, DRM_MODE_PROP_ATOMIC,
+ "BYPASS");
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->bypass_property = prop;
+ drm_object_attach_property(&colorop->base,
+ colorop->bypass_property,
+ 1);
+
/* curve_1d_type */
/* TODO move to mode_config? */
prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
@@ -100,6 +112,8 @@ static void
__drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colo
struct
drm_colorop_state *state)
{
memcpy(state, colorop->state, sizeof(*state));
+
+ state->bypass = true;
}
struct drm_colorop_state *
@@ -151,6 +165,7 @@ static void __drm_colorop_state_reset(struct
drm_colorop_state *colorop_state,
struct drm_colorop *colorop)
{
colorop_state->colorop = colorop;
+ colorop_state->bypass = true;
}
/**
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 965db7ec488f..053d1aa72e1b 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -44,6 +44,14 @@ struct drm_colorop_state {
/* colorop properties */
+ /**
+* @bypass:
+*
+* True if colorop shall be bypassed. False if colorop is
+* enabled.
+*/
+ bool bypass;
+
/**
* @curve_1d_type:
*
@@ -131,6 +139,18 @@ struct drm_colorop {
*/
struct drm_property *type_property;
+ /**
+* @bypass_property:
+*
+* Boolean property to control enablement of the color
+* operation. Setting bypass to "true" shall always be supported
+* in order to allow compositors to quickly fall back to
+* alternate methods of color processing. This is important
+* since setting color operations can fail due to unique
+* HW constraints.
+*/
+ struct drm_property *bypass_property;
+
/**
* @curve_1d_type:
*
--
2.42.1
[RFC PATCH v3 02/23] drm: Add helper for conversion from signed-magnitude
CTM values are defined as signed-magnitude values. Add
a helper that converts from CTM signed-magnitude fixed
point value to the twos-complement value used by
drm_fixed.
Signed-off-by: Harry Wentland
---
include/drm/drm_fixed.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/drm/drm_fixed.h b/include/drm/drm_fixed.h
index 0c9f917a4d4b..cb842ba80ddd 100644
--- a/include/drm/drm_fixed.h
+++ b/include/drm/drm_fixed.h
@@ -78,6 +78,24 @@ static inline u32 dfixed_div(fixed20_12 A, fixed20_12 B)
#define DRM_FIXED_EPSILON 1LL
#define DRM_FIXED_ALMOST_ONE (DRM_FIXED_ONE - DRM_FIXED_EPSILON)
+/**
+ * @drm_sm2fixp
+ *
+ * Convert a 1.31.32 signed-magnitude fixed point to 32.32
+ * 2s-complement fixed point
+ *
+ * @return s64 2s-complement fixed point
+ */
+static inline s64 drm_sm2fixp(__u64 a)
+{
+ if ((a & (1LL << 63))) {
+ return -(a & 0x7fffll);
+ } else {
+ return a;
+ }
+
+}
+
static inline s64 drm_int2fixp(int a)
{
return ((s64)a) << DRM_FIXED_POINT;
--
2.42.1
[RFC PATCH v3 06/23] drm/doc/rfc: Describe why prescriptive color pipeline is needed
v3: - Describe DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE (Sebastian) - Ask for clear documentation of colorop behavior (Sebastian) v2: - Update colorop visualizations to match reality (Sebastian, Alex Hung) - Updated wording (Pekka) - Change BYPASS wording to make it non-mandatory (Sebastian) - Drop cover-letter-like paragraph from COLOR_PIPELINE Plane Property section (Pekka) - Use PQ EOTF instead of its inverse in Pipeline Programming example (Melissa) - Add "Driver Implementer's Guide" section (Pekka) - Add "Driver Forward/Backward Compatibility" section (Sebastian, Pekka) Signed-off-by: Harry Wentland --- Documentation/gpu/rfc/color_pipeline.rst | 352 +++ 1 file changed, 352 insertions(+) create mode 100644 Documentation/gpu/rfc/color_pipeline.rst diff --git a/Documentation/gpu/rfc/color_pipeline.rst b/Documentation/gpu/rfc/color_pipeline.rst new file mode 100644 index ..efc70570a592 --- /dev/null +++ b/Documentation/gpu/rfc/color_pipeline.rst @@ -0,0 +1,352 @@ + +Linux Color Pipeline API + + +What problem are we solving? + + +We would like to support pre-, and post-blending complex color +transformations in display controller hardware in order to allow for +HW-supported HDR use-cases, as well as to provide support to +color-managed applications, such as video or image editors. + +It is possible to support an HDR output on HW supporting the Colorspace +and HDR Metadata drm_connector properties, but that requires the +compositor or application to render and compose the content into one +final buffer intended for display. Doing so is costly. + +Most modern display HW offers various 1D LUTs, 3D LUTs, matrices, and other +operations to support color transformations. These operations are often +implemented in fixed-function HW and therefore much more power efficient than +performing similar operations via shaders or CPU. + +We would like to make use of this HW functionality to support complex color +transformations with no, or minimal CPU or shader load. + + +How are other OSes solving this problem? + + +The most widely supported use-cases regard HDR content, whether video or +gaming. + +Most OSes will specify the source content format (color gamut, encoding transfer +function, and other metadata, such as max and average light levels) to a driver. +Drivers will then program their fixed-function HW accordingly to map from a +source content buffer's space to a display's space. + +When fixed-function HW is not available the compositor will assemble a shader to +ask the GPU to perform the transformation from the source content format to the +display's format. + +A compositor's mapping function and a driver's mapping function are usually +entirely separate concepts. On OSes where a HW vendor has no insight into +closed-source compositor code such a vendor will tune their color management +code to visually match the compositor's. On other OSes, where both mapping +functions are open to an implementer they will ensure both mappings match. + +This results in mapping algorithm lock-in, meaning that no-one alone can +experiment with or introduce new mapping algorithms and achieve +consistent results regardless of which implementation path is taken. + +Why is Linux different? +=== + +Unlike other OSes, where there is one compositor for one or more drivers, on +Linux we have a many-to-many relationship. Many compositors; many drivers. +In addition each compositor vendor or community has their own view of how +color management should be done. This is what makes Linux so beautiful. + +This means that a HW vendor can now no longer tune their driver to one +compositor, as tuning it to one could make it look fairly different from +another compositor's color mapping. + +We need a better solution. + + +Descriptive API +=== + +An API that describes the source and destination colorspaces is a descriptive +API. It describes the input and output color spaces but does not describe +how precisely they should be mapped. Such a mapping includes many minute +design decision that can greatly affect the look of the final result. + +It is not feasible to describe such mapping with enough detail to ensure the +same result from each implementation. In fact, these mappings are a very active +research area. + + +Prescriptive API + + +A prescriptive API describes not the source and destination colorspaces. It +instead prescribes a recipe for how to manipulate pixel values to arrive at the +desired outcome. + +This recipe is generally an ordered list of straight-forward operations, +with clear mathematical definitions, such as 1D LUTs, 3D LUTs, matrices, +or other operations that can be described in a precise manner. + + +The Color Pipeline API +== + +HW color management pipelines can significantly differ between H
[RFC PATCH v3 03/23] drm/vkms: Create separate Kconfig file for VKMS
This aligns with most other DRM drivers and will allow us to add new VKMS config options without polluting the DRM Kconfig. v3: - Change SPDX to GPL-2.0-only to match DRM KConfig SPDX (Simon) Signed-off-by: Harry Wentland Reviewed-by: Simon Ser --- drivers/gpu/drm/Kconfig | 14 +- drivers/gpu/drm/vkms/Kconfig | 15 +++ 2 files changed, 16 insertions(+), 13 deletions(-) create mode 100644 drivers/gpu/drm/vkms/Kconfig diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 48ca28a2e4ff..61ebd682c9b0 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -286,19 +286,7 @@ config DRM_VGEM as used by Mesa's software renderer for enhanced performance. If M is selected the module will be called vgem. -config DRM_VKMS - tristate "Virtual KMS (EXPERIMENTAL)" - depends on DRM && MMU - select DRM_KMS_HELPER - select DRM_GEM_SHMEM_HELPER - select CRC32 - default n - help - Virtual Kernel Mode-Setting (VKMS) is used for testing or for - running GPU in a headless machines. Choose this option to get - a VKMS. - - If M is selected the module will be called vkms. +source "drivers/gpu/drm/vkms/Kconfig" source "drivers/gpu/drm/exynos/Kconfig" diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig new file mode 100644 index ..b9ecdebecb0b --- /dev/null +++ b/drivers/gpu/drm/vkms/Kconfig @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: GPL-2.0-only + +config DRM_VKMS + tristate "Virtual KMS (EXPERIMENTAL)" + depends on DRM && MMU + select DRM_KMS_HELPER + select DRM_GEM_SHMEM_HELPER + select CRC32 + default n + help + Virtual Kernel Mode-Setting (VKMS) is used for testing or for + running GPU in a headless machines. Choose this option to get + a VKMS. + + If M is selected the module will be called vkms. -- 2.42.1
[RFC PATCH v3 11/23] drm/colorop: Add NEXT property
We'll construct color pipelines out of drm_colorop by
chaining them via the NEXT pointer. NEXT will point to
the next drm_colorop in the pipeline, or by 0 if we're
at the end of the pipeline.
v3:
- Add next pointer to colorop to be used by drivers
and in DRM core
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_colorop.c | 29 +
include/drm/drm_colorop.h | 20
2 files changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 17ba11ae205b..e62acf68bf9e 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -58,6 +58,7 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->dev = dev;
colorop->type = type;
colorop->plane = plane;
+ colorop->next = NULL;
list_add_tail(&colorop->head, &config->colorop_list);
colorop->index = config->num_colorop++;
@@ -104,6 +105,15 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->curve_1d_type_property,
0);
+ prop = drm_property_create_object(dev, DRM_MODE_PROP_IMMUTABLE |
DRM_MODE_PROP_ATOMIC,
+ "NEXT", DRM_MODE_OBJECT_COLOROP);
+ if (!prop)
+ return -ENOMEM;
+ colorop->next_property = prop;
+ drm_object_attach_property(&colorop->base,
+ colorop->next_property,
+ 0);
+
return ret;
}
EXPORT_SYMBOL(drm_colorop_init);
@@ -245,3 +255,22 @@ const char *drm_get_colorop_curve_1d_type_name(enum
drm_colorop_curve_1d_type ty
return colorop_curve_1d_type_name[type];
}
+
+/**
+ * drm_colorop_set_next_property - sets the next pointer
+ * @colorop: drm colorop
+ * @next: next colorop
+ *
+ * Should be used when constructing the color pipeline
+ */
+void drm_colorop_set_next_property(struct drm_colorop *colorop, struct
drm_colorop *next)
+{
+ if (!colorop->next_property)
+ return;
+
+ drm_object_property_set_value(&colorop->base,
+ colorop->next_property,
+ next->base.id);
+ colorop->next = next;
+}
+EXPORT_SYMBOL(drm_colorop_set_next_property);
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 053d1aa72e1b..c44f076a0606 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -131,6 +131,14 @@ struct drm_colorop {
*/
enum drm_colorop_type type;
+ /**
+* @next:
+*
+* Read-only
+* Pointer to next drm_colorop in pipeline
+*/
+ struct drm_colorop *next;
+
/**
* @type_property:
*
@@ -158,10 +166,20 @@ struct drm_colorop {
*/
struct drm_property *curve_1d_type_property;
+ /**
+* @next_property
+*
+* Read-only property to next colorop in the pipeline
+*/
+ struct drm_property *next_property;
+
};
#define obj_to_colorop(x) container_of(x, struct drm_colorop, base)
+
+
+
/**
* drm_crtc_find - look up a Colorop object from its ID
* @dev: DRM device
@@ -208,5 +226,7 @@ static inline unsigned int drm_colorop_index(const struct
drm_colorop *colorop)
#define drm_for_each_colorop(colorop, dev) \
list_for_each_entry(colorop, &(dev)->mode_config.colorop_list, head)
+void drm_colorop_set_next_property(struct drm_colorop *colorop, struct
drm_colorop *next);
+
#endif /* __DRM_COLOROP_H__ */
--
2.42.1
[RFC PATCH v3 04/23] drm/vkms: Add kunit tests for VKMS LUT handling
Debugging LUT math is much easier when we can unit test
it. Add kunit functionality to VKMS and add tests for
- get_lut_index
- lerp_u16
v3:
- Use include way of testing static functions (Arthur)
Signed-off-by: Harry Wentland
Cc: Arthur Grillo
---
drivers/gpu/drm/vkms/Kconfig | 5 ++
drivers/gpu/drm/vkms/tests/.kunitconfig | 4 ++
drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 62 +++
drivers/gpu/drm/vkms/vkms_composer.c | 8 ++-
4 files changed, 77 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_tests.c
diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
index b9ecdebecb0b..c1f8b343ff0e 100644
--- a/drivers/gpu/drm/vkms/Kconfig
+++ b/drivers/gpu/drm/vkms/Kconfig
@@ -13,3 +13,8 @@ config DRM_VKMS
a VKMS.
If M is selected the module will be called vkms.
+
+config DRM_VKMS_KUNIT_TESTS
+ tristate "Tests for VKMS" if !KUNIT_ALL_TESTS
+ depends on DRM_VKMS && KUNIT
+ default KUNIT_ALL_TESTS
diff --git a/drivers/gpu/drm/vkms/tests/.kunitconfig
b/drivers/gpu/drm/vkms/tests/.kunitconfig
new file mode 100644
index ..70e378228cbd
--- /dev/null
+++ b/drivers/gpu/drm/vkms/tests/.kunitconfig
@@ -0,0 +1,4 @@
+CONFIG_KUNIT=y
+CONFIG_DRM=y
+CONFIG_DRM_VKMS=y
+CONFIG_DRM_VKMS_KUNIT_TESTS=y
diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
new file mode 100644
index ..b995114cf6b8
--- /dev/null
+++ b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#include
+
+#include
+
+#define TEST_LUT_SIZE 16
+
+static struct drm_color_lut test_linear_array[TEST_LUT_SIZE] = {
+ { 0x0, 0x0, 0x0, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+ { 0x, 0x, 0x, 0 },
+};
+
+const struct vkms_color_lut test_linear_lut = {
+ .base = test_linear_array,
+ .lut_length = TEST_LUT_SIZE,
+ .channel_value2index_ratio = 0xf000fll
+};
+
+
+static void vkms_color_test_get_lut_index(struct kunit *test)
+{
+ int i;
+
+ KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&test_linear_lut,
test_linear_array[0].red)), 0);
+
+ for (i = 0; i < TEST_LUT_SIZE; i++)
+ KUNIT_EXPECT_EQ(test,
drm_fixp2int_ceil(get_lut_index(&test_linear_lut, test_linear_array[i].red)),
i);
+}
+
+static void vkms_color_test_lerp(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, 0x8000), 0x8);
+}
+
+static struct kunit_case vkms_color_test_cases[] = {
+ KUNIT_CASE(vkms_color_test_get_lut_index),
+ KUNIT_CASE(vkms_color_test_lerp),
+ {}
+};
+
+static struct kunit_suite vkms_color_test_suite = {
+ .name = "vkms-color",
+ .test_cases = vkms_color_test_cases,
+};
+kunit_test_suite(vkms_color_test_suite);
+
+MODULE_LICENSE("GPL");
\ No newline at end of file
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 3c99fb8b54e2..6f942896036e 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -91,7 +91,7 @@ static void fill_background(const struct pixel_argb_u16
*background_color,
}
// lerp(a, b, t) = a + (b - a) * t
-static u16 lerp_u16(u16 a, u16 b, s64 t)
+u16 lerp_u16(u16 a, u16 b, s64 t)
{
s64 a_fp = drm_int2fixp(a);
s64 b_fp = drm_int2fixp(b);
@@ -101,7 +101,7 @@ static u16 lerp_u16(u16 a, u16 b, s64 t)
return drm_fixp2int(a_fp + delta);
}
-static s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value)
+s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value)
{
s64 color_channel_fp = drm_int2fixp(channel_value);
@@ -429,3 +429,7 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char
*src_name)
return ret;
}
+
+#ifdef CONFIG_DRM_VKMS_KUNIT_TESTS
+#include "tests/vkms_color_tests.c"
+#endif
--
2.42.1
[RFC PATCH v3 12/23] drm/colorop: Add atomic state print for drm_colorop
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_atomic.c | 29 +
include/drm/drm_colorop.h| 5 +
2 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 59e1797d1ca8..ccf26b034433 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -783,6 +783,19 @@ static int drm_atomic_plane_check(const struct
drm_plane_state *old_plane_state,
return 0;
}
+
+
+static void drm_atomic_colorop_print_state(struct drm_printer *p,
+ const struct drm_colorop_state *state)
+{
+ struct drm_colorop *colorop = state->colorop;
+
+ drm_printf(p, "colorop[%u]:\n", colorop->base.id);
+ drm_printf(p, "\ttype=%s\n", drm_get_colorop_type_name(colorop->type));
+ drm_printf(p, "\tbypass=%u\n", state->bypass);
+ drm_printf(p, "\tcurve_1d_type=%s\n",
drm_get_colorop_curve_1d_type_name(state->curve_1d_type));
+}
+
static void drm_atomic_plane_print_state(struct drm_printer *p,
const struct drm_plane_state *state)
{
@@ -803,6 +816,13 @@ static void drm_atomic_plane_print_state(struct
drm_printer *p,
drm_get_color_encoding_name(state->color_encoding));
drm_printf(p, "\tcolor-range=%s\n",
drm_get_color_range_name(state->color_range));
+#if 0
+ drm_printf(p, "\tcolor-pipeline=%s\n",
+ drm_get_color_pipeline_name(state->color_pipeline));
+#else
+ drm_printf(p, "\tcolor-pipeline=%d\n",
+ state->color_pipeline ? state->color_pipeline->base.id : 0);
+#endif
if (plane->funcs->atomic_print_state)
plane->funcs->atomic_print_state(p, state);
@@ -1839,6 +1859,7 @@ static void __drm_state_dump(struct drm_device *dev,
struct drm_printer *p,
bool take_locks)
{
struct drm_mode_config *config = &dev->mode_config;
+ struct drm_colorop *colorop;
struct drm_plane *plane;
struct drm_crtc *crtc;
struct drm_connector *connector;
@@ -1847,6 +1868,14 @@ static void __drm_state_dump(struct drm_device *dev,
struct drm_printer *p,
if (!drm_drv_uses_atomic_modeset(dev))
return;
+ list_for_each_entry(colorop, &config->colorop_list, head) {
+ if (take_locks)
+ drm_modeset_lock(&colorop->plane->mutex, NULL);
+ drm_atomic_colorop_print_state(p, colorop->state);
+ if (take_locks)
+ drm_modeset_unlock(&colorop->plane->mutex);
+ }
+
list_for_each_entry(plane, &config->plane_list, head) {
if (take_locks)
drm_modeset_lock(&plane->mutex, NULL);
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index c44f076a0606..fbf7c0489fc8 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -226,6 +226,11 @@ static inline unsigned int drm_colorop_index(const struct
drm_colorop *colorop)
#define drm_for_each_colorop(colorop, dev) \
list_for_each_entry(colorop, &(dev)->mode_config.colorop_list, head)
+const char *drm_get_color_pipeline_name(struct drm_colorop *colorop);
+
+const char *drm_get_colorop_type_name(enum drm_colorop_type type);
+const char *drm_get_colorop_curve_1d_type_name(enum drm_colorop_curve_1d_type
type);
+
void drm_colorop_set_next_property(struct drm_colorop *colorop, struct
drm_colorop *next);
--
2.42.1
[RFC PATCH v3 09/23] drm/color: Add 1D Curve subtype
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_atomic_uapi.c | 18 ++
drivers/gpu/drm/drm_colorop.c | 39 +++
include/drm/drm_colorop.h | 20
3 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index f22bd8671236..52b9b48e5757 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -670,11 +670,17 @@ static int drm_atomic_colorop_set_property(struct
drm_colorop *colorop,
struct drm_colorop_state *state, struct drm_file *file_priv,
struct drm_property *property, uint64_t val)
{
- drm_dbg_atomic(colorop->dev,
- "[COLOROP:%d] unknown property [PROP:%d:%s]]\n",
- colorop->base.id,
- property->base.id, property->name);
- return -EINVAL;
+ if (property == colorop->curve_1d_type_property) {
+ state->curve_1d_type = val;
+ } else {
+ drm_dbg_atomic(colorop->dev,
+ "[COLOROP:%d:%d] unknown property
[PROP:%d:%s]]\n",
+ colorop->base.id, colorop->type,
+ property->base.id, property->name);
+ return -EINVAL;
+ }
+
+ return 0;
}
static int
@@ -684,6 +690,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
{
if (property == colorop->type_property) {
*val = colorop->type;
+ } else if (property == colorop->curve_1d_type_property) {
+ *val = state->curve_1d_type;
} else {
return -EINVAL;
}
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 902b13e300fd..b1c271f90a16 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -36,6 +36,11 @@ static const struct drm_prop_enum_list
drm_colorop_type_enum_list[] = {
{ DRM_COLOROP_1D_CURVE, "1D Curve" },
};
+static const struct drm_prop_enum_list drm_colorop_curve_1d_type_enum_list[] =
{
+ { DRM_COLOROP_1D_CURVE_SRGB_EOTF, "sRGB EOTF" },
+ { DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF, "sRGB Inverse EOTF" },
+};
+
/* Init Helpers */
int drm_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
@@ -73,6 +78,20 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->type_property,
colorop->type);
+ /* curve_1d_type */
+ /* TODO move to mode_config? */
+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
+ "CURVE_1D_TYPE",
+ drm_colorop_curve_1d_type_enum_list,
+
ARRAY_SIZE(drm_colorop_curve_1d_type_enum_list));
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->curve_1d_type_property = prop;
+ drm_object_attach_property(&colorop->base,
+ colorop->curve_1d_type_property,
+ 0);
+
return ret;
}
EXPORT_SYMBOL(drm_colorop_init);
@@ -177,6 +196,11 @@ static const char * const colorop_type_name[] = {
[DRM_COLOROP_1D_CURVE] = "1D Curve",
};
+static const char * const colorop_curve_1d_type_name[] = {
+ [DRM_COLOROP_1D_CURVE_SRGB_EOTF] = "sRGB EOTF",
+ [DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF] = "sRGB Inverse EOTF",
+};
+
/**
* drm_get_colorop_type_name - return a string for colorop type
* @type: colorop type to compute name of
@@ -191,3 +215,18 @@ const char *drm_get_colorop_type_name(enum
drm_colorop_type type)
return colorop_type_name[type];
}
+
+/**
+ * drm_get_colorop_curve_1d_type_name - return a string for 1D curve type
+ * @range: 1d curve type to compute name of
+ *
+ * In contrast to the other drm_get_*_name functions this one here returns a
+ * const pointer and hence is threadsafe.
+ */
+const char *drm_get_colorop_curve_1d_type_name(enum drm_colorop_curve_1d_type
type)
+{
+ if (WARN_ON(type >= ARRAY_SIZE(colorop_curve_1d_type_name)))
+ return "unknown";
+
+ return colorop_curve_1d_type_name[type];
+}
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 0386440afe40..965db7ec488f 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -30,6 +30,11 @@
#include
#include
+enum drm_colorop_curve_1d_type {
+ DRM_COLOROP_1D_CURVE_SRGB_EOTF,
+ DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF
+};
+
/**
* struct drm_colorop_state - mutable colorop state
*/
@@ -39,6 +44,13 @@ struct drm_colorop_state {
/* colorop properties */
+ /**
+* @curve_1d_type:
+*
+* Type of 1D curve.
+*/
+ enum drm_colorop_curve_1d_type curve_1d_type;
+
/** @state: backpointer t
[RFC PATCH v3 14/23] drm/colorop: Add NEXT to colorop state print
v3: - Read NEXT ID from drm_colorop's next pointer Signed-off-by: Harry Wentland --- drivers/gpu/drm/drm_atomic.c | 1 + include/drm/drm_colorop.h| 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index cf3cb6d1239f..02bb071f735c 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -794,6 +794,7 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p, drm_printf(p, "\ttype=%s\n", drm_get_colorop_type_name(colorop->type)); drm_printf(p, "\tbypass=%u\n", state->bypass); drm_printf(p, "\tcurve_1d_type=%s\n", drm_get_colorop_curve_1d_type_name(state->curve_1d_type)); + drm_printf(p, "\tnext=%d\n", colorop->next ? colorop->next->base.id : 0); } static void drm_atomic_plane_print_state(struct drm_printer *p, diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h index fbf7c0489fc8..13acc9a6ac38 100644 --- a/include/drm/drm_colorop.h +++ b/include/drm/drm_colorop.h @@ -232,6 +232,8 @@ const char *drm_get_colorop_type_name(enum drm_colorop_type type); const char *drm_get_colorop_curve_1d_type_name(enum drm_colorop_curve_1d_type type); void drm_colorop_set_next_property(struct drm_colorop *colorop, struct drm_colorop *next); +uint32_t drm_colorop_get_next_property(struct drm_colorop *colorop); +struct drm_colorop *drm_colorop_get_next(struct drm_colorop *colorop); #endif /* __DRM_COLOROP_H__ */ -- 2.42.1
[RFC PATCH v3 16/23] drm/vkms: Add kunit tests for linear and sRGB LUTs
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 37 ++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
index b995114cf6b8..ad4c2f72fd1e 100644
--- a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
+++ b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
@@ -31,7 +31,6 @@ const struct vkms_color_lut test_linear_lut = {
.channel_value2index_ratio = 0xf000fll
};
-
static void vkms_color_test_get_lut_index(struct kunit *test)
{
int i;
@@ -40,6 +39,19 @@ static void vkms_color_test_get_lut_index(struct kunit *test)
for (i = 0; i < TEST_LUT_SIZE; i++)
KUNIT_EXPECT_EQ(test,
drm_fixp2int_ceil(get_lut_index(&test_linear_lut, test_linear_array[i].red)),
i);
+
+ KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&srgb_eotf, 0x0)),
0x0);
+ KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
0x0)), 0x0);
+ KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
0x101)), 0x1);
+ KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
0x202)), 0x2);
+
+ KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&srgb_inv_eotf, 0x0)),
0x0);
+ KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_inv_eotf,
0x0)), 0x0);
+ KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_inv_eotf,
0x101)), 0x1);
+ KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_inv_eotf,
0x202)), 0x2);
+
+ KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
0xfefe)), 0xfe);
+ KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
0x)), 0xff);
}
static void vkms_color_test_lerp(struct kunit *test)
@@ -47,9 +59,32 @@ static void vkms_color_test_lerp(struct kunit *test)
KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x10, 0x8000), 0x8);
}
+static void vkms_color_test_linear(struct kunit *test)
+{
+ for (int i = 0; i < LUT_SIZE; i++) {
+ int linear = apply_lut_to_channel_value(&linear_eotf, i *
0x101, LUT_RED);
+ KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(linear, 0x101), i);
+ }
+}
+
+static void vkms_color_srgb_inv_srgb(struct kunit *test)
+{
+ u16 srgb, final;
+
+ for (int i = 0; i < LUT_SIZE; i++) {
+ srgb = apply_lut_to_channel_value(&srgb_eotf, i * 0x101,
LUT_RED);
+ final = apply_lut_to_channel_value(&srgb_inv_eotf, srgb,
LUT_RED);
+
+ KUNIT_EXPECT_GE(test, final / 0x101, i-1);
+ KUNIT_EXPECT_LE(test, final / 0x101, i+1);
+ }
+}
+
static struct kunit_case vkms_color_test_cases[] = {
KUNIT_CASE(vkms_color_test_get_lut_index),
KUNIT_CASE(vkms_color_test_lerp),
+ KUNIT_CASE(vkms_color_test_linear),
+ KUNIT_CASE(vkms_color_srgb_inv_srgb),
{}
};
--
2.42.1
[RFC PATCH v3 19/23] drm/vkms: Pull apply_colorop out of pre_blend_color_transform
The if/switch statement is bound to grow with more types and
subtypes. Pull this out into its own funcion to make things more
manageable and readable.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/vkms/vkms_composer.c | 48
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index be42756e300a..9010415e4bd6 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -164,6 +164,31 @@ static void apply_lut(const struct vkms_crtc_state
*crtc_state, struct line_buff
}
}
+static void apply_colorop(struct pixel_argb_u16 *pixel, struct drm_colorop
*colorop)
+{
+ /* TODO is this right? */
+ struct drm_colorop_state *colorop_state = colorop->state;
+
+ if (colorop->type == DRM_COLOROP_1D_CURVE) {
+ switch (colorop_state->curve_1d_type) {
+ case DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF:
+ pixel->r =
apply_lut_to_channel_value(&srgb_inv_eotf, pixel->r, LUT_RED);
+ pixel->g =
apply_lut_to_channel_value(&srgb_inv_eotf, pixel->g, LUT_GREEN);
+ pixel->b =
apply_lut_to_channel_value(&srgb_inv_eotf, pixel->b, LUT_BLUE);
+ break;
+ case DRM_COLOROP_1D_CURVE_SRGB_EOTF:
+ pixel->r =
apply_lut_to_channel_value(&srgb_eotf, pixel->r, LUT_RED);
+ pixel->g =
apply_lut_to_channel_value(&srgb_eotf, pixel->g, LUT_GREEN);
+ pixel->b =
apply_lut_to_channel_value(&srgb_eotf, pixel->b, LUT_BLUE);
+ break;
+ default:
+ DRM_DEBUG_DRIVER("unkown colorop 1D curve type
%d\n", colorop_state->curve_1d_type);
+ break;
+ }
+ }
+
+}
+
static void pre_blend_color_transform(const struct vkms_plane_state
*plane_state, struct line_buffer *output_buffer)
{
struct drm_colorop *colorop = plane_state->base.base.color_pipeline;
@@ -180,26 +205,9 @@ static void pre_blend_color_transform(const struct
vkms_plane_state *plane_state
if (!colorop_state)
return;
- for (size_t x = 0; x < output_buffer->n_pixels; x++) {
- struct pixel_argb_u16 *pixel =
&output_buffer->pixels[x];
-
- if (colorop->type == DRM_COLOROP_1D_CURVE &&
- colorop_state->bypass == false) {
- switch (colorop_state->curve_1d_type) {
- case DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF:
- pixel->r =
apply_lut_to_channel_value(&srgb_inv_eotf, pixel->r, LUT_RED);
- pixel->g =
apply_lut_to_channel_value(&srgb_inv_eotf, pixel->g, LUT_GREEN);
- pixel->b =
apply_lut_to_channel_value(&srgb_inv_eotf, pixel->b, LUT_BLUE);
- break;
- case DRM_COLOROP_1D_CURVE_SRGB_EOTF:
- default:
- pixel->r =
apply_lut_to_channel_value(&srgb_eotf, pixel->r, LUT_RED);
- pixel->g =
apply_lut_to_channel_value(&srgb_eotf, pixel->g, LUT_GREEN);
- pixel->b =
apply_lut_to_channel_value(&srgb_eotf, pixel->b, LUT_BLUE);
- break;
- }
- }
- }
+ for (size_t x = 0; x < output_buffer->n_pixels; x++)
+ if (!colorop_state->bypass)
+ apply_colorop(&output_buffer->pixels[x],
colorop);
colorop = colorop->next;
}
--
2.42.1
[RFC PATCH v3 07/23] drm/colorop: Introduce new drm_colorop mode object
This patches introduces a new drm_colorop mode object. This
object represents color transformations and can be used to
define color pipelines.
We also introduce the drm_colorop_state here, as well as
various helpers and state tracking bits.
v3:
- Drop TODO for lock (it's handled in drm_modeset_drop_locks)
(Melissa)
- Don't get plane state when getting colorop state
- Make some functions static (kernel test robot)
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/Makefile| 1 +
drivers/gpu/drm/drm_atomic.c| 70 +
drivers/gpu/drm/drm_atomic_helper.c | 12 +++
drivers/gpu/drm/drm_atomic_uapi.c | 48 +
drivers/gpu/drm/drm_colorop.c | 152 +++
drivers/gpu/drm/drm_mode_config.c | 7 ++
drivers/gpu/drm/drm_plane_helper.c | 2 +-
include/drm/drm_atomic.h| 82 +++
include/drm/drm_atomic_uapi.h | 1 +
include/drm/drm_colorop.h | 157
include/drm/drm_mode_config.h | 18
include/drm/drm_plane.h | 2 +
include/uapi/drm/drm.h | 3 +
include/uapi/drm/drm_mode.h | 1 +
14 files changed, 555 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_colorop.c
create mode 100644 include/drm/drm_colorop.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 8e1bde059170..7ba67f9775e7 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,6 +16,7 @@ drm-y := \
drm_client.o \
drm_client_modeset.o \
drm_color_mgmt.o \
+ drm_colorop.o \
drm_connector.o \
drm_crtc.o \
drm_displayid.o \
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index f1a503aafe5a..6390443f1819 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#include "drm_crtc_internal.h"
#include "drm_internal.h"
@@ -108,6 +109,7 @@ void drm_atomic_state_default_release(struct
drm_atomic_state *state)
kfree(state->connectors);
kfree(state->crtcs);
kfree(state->planes);
+ kfree(state->colorops);
kfree(state->private_objs);
}
EXPORT_SYMBOL(drm_atomic_state_default_release);
@@ -139,6 +141,10 @@ drm_atomic_state_init(struct drm_device *dev, struct
drm_atomic_state *state)
sizeof(*state->planes), GFP_KERNEL);
if (!state->planes)
goto fail;
+ state->colorops = kcalloc(dev->mode_config.num_colorop,
+ sizeof(*state->colorops), GFP_KERNEL);
+ if (!state->colorops)
+ goto fail;
/*
* Because drm_atomic_state can be committed asynchronously we need our
@@ -250,6 +256,20 @@ void drm_atomic_state_default_clear(struct
drm_atomic_state *state)
state->planes[i].new_state = NULL;
}
+ for (i = 0; i < config->num_colorop; i++) {
+ struct drm_colorop *colorop = state->colorops[i].ptr;
+
+ if (!colorop)
+ continue;
+
+ drm_colorop_atomic_destroy_state(colorop,
+state->colorops[i].state);
+ state->colorops[i].ptr = NULL;
+ state->colorops[i].state = NULL;
+ state->colorops[i].old_state = NULL;
+ state->colorops[i].new_state = NULL;
+ }
+
for (i = 0; i < state->num_private_objs; i++) {
struct drm_private_obj *obj = state->private_objs[i].ptr;
@@ -571,6 +591,56 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_plane_state);
+
+/**
+ * drm_atomic_get_colorop_state - get colorop state
+ * @state: global atomic state object
+ * @colorop: colorop to get state object for
+ *
+ * This function returns the colorop state for the given colorop, allocating it
+ * if needed. It will also grab the relevant plane lock to make sure that the
+ * state is consistent.
+ *
+ * Returns:
+ *
+ * Either the allocated state or the error code encoded into the pointer. When
+ * the error is EDEADLK then the w/w mutex code has detected a deadlock and the
+ * entire atomic sequence must be restarted. All other errors are fatal.
+ */
+struct drm_colorop_state *
+drm_atomic_get_colorop_state(struct drm_atomic_state *state,
+struct drm_colorop *colorop)
+{
+ int ret, index = drm_colorop_index(colorop);
+ struct drm_colorop_state *colorop_state;
+
+ WARN_ON(!state->acquire_ctx);
+
+ colorop_state = drm_atomic_get_existing_colorop_state(state, colorop);
+ if (colorop_state)
+ return colorop_state;
+
+ ret = drm_modeset_lock(&colorop->plane->mutex, state->acquire_ctx);
+ if (ret)
+ return ERR_PTR(ret);
+
+ colorop_state = drm_atomic_helper_colorop
[RFC PATCH v3 17/23] drm/colorop: Introduce DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE
With the introduction of the pre-blending color pipeline we
can no longer have color operations that don't have a clear
position in the color pipeline. We deprecate all existing
plane properties. For upstream drivers those are:
- COLOR_ENCODING
- COLOR_RANGE
Userspace that registers with the
DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE will be permitted to set
the COLOR_PIPELINE plane property and drm_colorop properties.
But it won't be allowed to set COLOR_ENCODING and
COLOR_RANGE. Userspace that does not set this client cap
will not be permitted to touch the color pipeline.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_atomic_uapi.c | 23 ++-
drivers/gpu/drm/drm_ioctl.c | 7 +++
include/drm/drm_file.h| 7 +++
include/uapi/drm/drm.h| 15 +++
4 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index c6629fdaa114..69c56982e2d0 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -610,10 +610,26 @@ static int drm_atomic_plane_set_property(struct drm_plane
*plane,
} else if (property == plane->zpos_property) {
state->zpos = val;
} else if (property == plane->color_encoding_property) {
+ if (file_priv->plane_color_pipeline) {
+ drm_dbg_atomic(dev,
+ "Setting COLOR_PIPELINE plane property
not permitted when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n");
+ return -EINVAL;
+ }
state->color_encoding = val;
} else if (property == plane->color_range_property) {
+ if (file_priv->plane_color_pipeline) {
+ drm_dbg_atomic(dev,
+ "Setting COLOR_PIPELINE plane property
not permitted when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n");
+ return -EINVAL;
+ }
state->color_range = val;
} else if (property == plane->color_pipeline_property) {
+ if (!file_priv->plane_color_pipeline) {
+ drm_dbg_atomic(dev,
+ "Setting COLOR_PIPELINE plane property
not permitted unless DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set\n");
+ return -EINVAL;
+ }
+
/* find DRM colorop object */
struct drm_colorop *colorop = NULL;
colorop = drm_colorop_find(dev, file_priv, val);
@@ -1158,6 +1174,12 @@ int drm_atomic_set_property(struct drm_atomic_state
*state,
break;
}
case DRM_MODE_OBJECT_COLOROP: {
+ if (!file_priv->plane_color_pipeline) {
+ drm_dbg_atomic(prop->dev,
+ "[OBJECT:%d] is a colorop but
DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE not set\n",
+ obj->id);
+ ret = -EINVAL;
+ }
struct drm_colorop *colorop = obj_to_colorop(obj);
struct drm_colorop_state *colorop_state;
@@ -1170,7 +1192,6 @@ int drm_atomic_set_property(struct drm_atomic_state
*state,
ret = drm_atomic_colorop_set_property(colorop,
colorop_state, file_priv,
prop, prop_value);
-
break;
}
default:
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 44fda68c28ae..0d869658e13e 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -361,6 +361,13 @@ drm_setclientcap(struct drm_device *dev, void *data,
struct drm_file *file_priv)
return -EINVAL;
file_priv->writeback_connectors = req->value;
break;
+ case DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE:
+ if (!file_priv->atomic)
+ return -EINVAL;
+ if (req->value > 1)
+ return -EINVAL;
+ file_priv->plane_color_pipeline = req->value;
+ break;
default:
return -EINVAL;
}
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index e1b5b4282f75..bf11b646c898 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -204,6 +204,13 @@ struct drm_file {
*/
bool writeback_connectors;
+ /**
+* @plane_color_pipeline:
+*
+* True if client understands plane color pipelines
+*/
+ bool plane_color_pipeline;
+
/**
* @was_master:
*
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 841d393fb84e..2576b170e8d0 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -842,6 +842,21 @@ struct drm_get_cap {
*/
#define DRM_CLIENT_CAP_WRITEBACK_CON
[RFC PATCH v3 08/23] drm/colorop: Add TYPE property
Add a read-only TYPE property. The TYPE specifies the colorop
type, such as enumerated curve, 1D LUT, CTM, 3D LUT, PWL LUT,
etc.
v3:
- Make TYPE a range property
- Move enum drm_colorop_type to uapi header
- Fix drm_get_colorop_type_name description
For now we're only introducing an enumerated 1D LUT type to
illustrate the concept.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_atomic.c | 4 +--
drivers/gpu/drm/drm_atomic_uapi.c | 8 +-
drivers/gpu/drm/drm_colorop.c | 43 ++-
include/drm/drm_colorop.h | 17 +++-
include/uapi/drm/drm_mode.h | 4 +++
5 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 6390443f1819..59e1797d1ca8 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -634,8 +634,8 @@ drm_atomic_get_colorop_state(struct drm_atomic_state *state,
state->colorops[index].new_state = colorop_state;
colorop_state->state = state;
- drm_dbg_atomic(colorop->dev, "Added [COLOROP:%d] %p state to %p\n",
- colorop->base.id, colorop_state, state);
+ drm_dbg_atomic(colorop->dev, "Added [COLOROP:%d:%d] %p state to %p\n",
+ colorop->base.id, colorop->type, colorop_state, state);
return colorop_state;
}
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 21da1b327ee9..f22bd8671236 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -682,7 +682,13 @@ drm_atomic_colorop_get_property(struct drm_colorop
*colorop,
const struct drm_colorop_state *state,
struct drm_property *property, uint64_t *val)
{
- return -EINVAL;
+ if (property == colorop->type_property) {
+ *val = colorop->type;
+ } else {
+ return -EINVAL;
+ }
+
+ return 0;
}
static int drm_atomic_set_writeback_fb_for_connector(
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index a295ab96aee1..902b13e300fd 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -32,12 +32,17 @@
/* TODO big colorop doc, including properties, etc. */
+static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = {
+ { DRM_COLOROP_1D_CURVE, "1D Curve" },
+};
+
/* Init Helpers */
int drm_colorop_init(struct drm_device *dev, struct drm_colorop *colorop,
-struct drm_plane *plane)
+struct drm_plane *plane, enum drm_colorop_type type)
{
struct drm_mode_config *config = &dev->mode_config;
+ struct drm_property *prop;
int ret = 0;
ret = drm_mode_object_add(dev, &colorop->base, DRM_MODE_OBJECT_COLOROP);
@@ -46,12 +51,28 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->base.properties = &colorop->properties;
colorop->dev = dev;
+ colorop->type = type;
colorop->plane = plane;
list_add_tail(&colorop->head, &config->colorop_list);
colorop->index = config->num_colorop++;
/* add properties */
+
+ /* type */
+ prop = drm_property_create_range(dev,
+ DRM_MODE_PROP_IMMUTABLE,
+ "TYPE", type, type);
+
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->type_property = prop;
+
+ drm_object_attach_property(&colorop->base,
+ colorop->type_property,
+ colorop->type);
+
return ret;
}
EXPORT_SYMBOL(drm_colorop_init);
@@ -150,3 +171,23 @@ void drm_colorop_reset(struct drm_colorop *colorop)
__drm_colorop_reset(colorop, colorop->state);
}
EXPORT_SYMBOL(drm_colorop_reset);
+
+
+static const char * const colorop_type_name[] = {
+ [DRM_COLOROP_1D_CURVE] = "1D Curve",
+};
+
+/**
+ * drm_get_colorop_type_name - return a string for colorop type
+ * @type: colorop type to compute name of
+ *
+ * In contrast to the other drm_get_*_name functions this one here returns a
+ * const pointer and hence is threadsafe.
+ */
+const char *drm_get_colorop_type_name(enum drm_colorop_type type)
+{
+ if (WARN_ON(type >= ARRAY_SIZE(colorop_type_name)))
+ return "unknown";
+
+ return colorop_type_name[type];
+}
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 3dd169b0317d..0386440afe40 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -103,6 +103,21 @@ struct drm_colorop {
/** @properties: property tracking for this plane */
struct drm_object_properties properties;
+ /**
+* @type:
+*
+* Read-only
+* Type of color operation
+*/
+ enum drm_colorop_type type;
+
+ /**
+* @type_property:
+*
[RFC PATCH v3 13/23] drm/plane: Add COLOR PIPELINE property
We're adding a new enum COLOR PIPELINE property. This
property will have entries for each COLOR PIPELINE by
referencing the DRM object ID of the first drm_colorop
of the pipeline. 0 disables the entire COLOR PIPELINE.
Userspace can use this to discover the available color
pipelines, as well as set the desired one. The color
pipelines are programmed via properties on the actual
drm_colorop objects.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_atomic.c | 46 +++
drivers/gpu/drm/drm_atomic_state_helper.c | 5 +++
drivers/gpu/drm/drm_atomic_uapi.c | 44 ++
include/drm/drm_atomic_uapi.h | 2 +
include/drm/drm_plane.h | 8
5 files changed, 105 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index ccf26b034433..cf3cb6d1239f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1463,6 +1463,52 @@ drm_atomic_add_affected_planes(struct drm_atomic_state
*state,
}
EXPORT_SYMBOL(drm_atomic_add_affected_planes);
+/**
+ * drm_atomic_add_affected_colorops - add colorops for plane
+ * @state: atomic state
+ * @plane: DRM plane
+ *
+ * This function walks the current configuration and adds all colorops
+ * currently used by @plane to the atomic configuration @state. This is useful
+ * when an atomic commit also needs to check all currently enabled colorop on
+ * @plane, e.g. when changing the mode. It's also useful when re-enabling a
plane
+ * to avoid special code to force-enable all colorops.
+ *
+ * Since acquiring a colorop state will always also acquire the w/w mutex of
the
+ * current plane for that colorop (if there is any) adding all the colorop
states for
+ * a plane will not reduce parallelism of atomic updates.
+ *
+ * Returns:
+ * 0 on success or can fail with -EDEADLK or -ENOMEM. When the error is EDEADLK
+ * then the w/w mutex code has detected a deadlock and the entire atomic
+ * sequence must be restarted. All other errors are fatal.
+ */
+int
+drm_atomic_add_affected_colorops(struct drm_atomic_state *state,
+struct drm_plane *plane)
+{
+ struct drm_colorop *colorop;
+ struct drm_colorop_state *colorop_state;
+
+ WARN_ON(!drm_atomic_get_new_plane_state(state, plane));
+
+ drm_dbg_atomic(plane->dev,
+ "Adding all current colorops for [plane:%d:%s] to %p\n",
+ plane->base.id, plane->name, state);
+
+ drm_for_each_colorop(colorop, plane->dev) {
+ if (colorop->plane != plane)
+ continue;
+
+ colorop_state = drm_atomic_get_colorop_state(state, colorop);
+ if (IS_ERR(colorop_state))
+ return PTR_ERR(colorop_state);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_add_affected_colorops);
+
/**
* drm_atomic_check_only - check whether a given config would work
* @state: atomic configuration to check
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 784e63d70a42..3c5f2c8e33d0 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -267,6 +267,11 @@ void __drm_atomic_helper_plane_state_reset(struct
drm_plane_state *plane_state,
plane_state->color_range = val;
}
+ if (plane->color_pipeline_property) {
+ /* default is always NULL, i.e., bypass */
+ plane_state->color_pipeline = NULL;
+ }
+
if (plane->zpos_property) {
if (!drm_object_property_get_default_value(&plane->base,
plane->zpos_property,
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index a8f7a8a6639a..c6629fdaa114 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -256,6 +256,38 @@ drm_atomic_set_fb_for_plane(struct drm_plane_state
*plane_state,
}
EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
+
+/**
+ * drm_atomic_set_colorop_for_plane - set colorop for plane
+ * @plane_state: atomic state object for the plane
+ * @colorop: colorop to use for the plane
+ *
+ * Changing the assigned framebuffer for a plane requires us to grab a
reference
+ * to the new fb and drop the reference to the old fb, if there is one. This
+ * function takes care of all these details besides updating the pointer in the
+ * state object itself.
+ */
+void
+drm_atomic_set_colorop_for_plane(struct drm_plane_state *plane_state,
+struct drm_colorop *colorop)
+{
+ struct drm_plane *plane = plane_state->plane;
+
+ if (colorop)
+ drm_dbg_atomic(plane->dev,
+ "Set [COLOROP:%d] for [PLANE:%d:%s] state %p\n",
+ colorop->base.id, plane->base.id, plane->name,
+
[RFC PATCH v3 18/23] drm/colorop: Add 3x4 CTM type
This type is used to support a 3x4 matrix in colorops. A 3x4
matrix uses the last column as a "bias" column. Some HW exposes
support for 3x4. The calculation looks like:
out matrixin
|R| |0 1 2 3 | | R |
|G| = |4 5 6 7 | x | G |
|B| |8 9 10 12| | B |
|1.0|
This is also the first colorop where we need a blob property to
program the property. For that we'll introduce a new DATA
property that can be used by all colorop TYPEs requiring a
blob. The way a DATA blob is read depends on the TYPE of
the colorop.
We only create the DATA property for property types that
need it.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/drm_atomic_uapi.c | 30 ++
drivers/gpu/drm/drm_colorop.c | 16
include/drm/drm_colorop.h | 19 +++
include/uapi/drm/drm_mode.h | 19 ++-
4 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index 69c56982e2d0..564bca68f652 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -725,6 +725,31 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
return 0;
}
+static int drm_atomic_color_set_data_property(struct drm_colorop *colorop,
+ struct drm_colorop_state *state,
+ struct drm_property *property, uint64_t val)
+{
+ ssize_t elem_size = -1;
+ ssize_t size = -1;
+ bool replaced;
+
+
+ switch (colorop->type) {
+ case DRM_COLOROP_CTM_3X4:
+ size = sizeof(struct drm_color_ctm_3x4);
+ break;
+ default:
+ /* should never get here */
+ return -EINVAL;
+ }
+
+ return drm_atomic_replace_property_blob_from_id(colorop->dev,
+ &state->data,
+ val,
+ size,
+ elem_size,
+ &replaced);
+}
static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
struct drm_colorop_state *state, struct drm_file *file_priv,
@@ -734,6 +759,9 @@ static int drm_atomic_colorop_set_property(struct
drm_colorop *colorop,
state->bypass = val;
} else if (property == colorop->curve_1d_type_property) {
state->curve_1d_type = val;
+ } else if (property == colorop->data_property) {
+ return drm_atomic_color_set_data_property(colorop,
+ state, property, val);
} else {
drm_dbg_atomic(colorop->dev,
"[COLOROP:%d:%d] unknown property
[PROP:%d:%s]]\n",
@@ -756,6 +784,8 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
*val = state->bypass;
} else if (property == colorop->curve_1d_type_property) {
*val = state->curve_1d_type;
+ } else if (property == colorop->data_property) {
+ *val = (state->data) ? state->data->base.id : 0;
} else {
return -EINVAL;
}
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index e62acf68bf9e..67e6efc90803 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -34,6 +34,7 @@
static const struct drm_prop_enum_list drm_colorop_type_enum_list[] = {
{ DRM_COLOROP_1D_CURVE, "1D Curve" },
+ { DRM_COLOROP_CTM_3X4, "3x4 Matrix"}
};
static const struct drm_prop_enum_list drm_colorop_curve_1d_type_enum_list[] =
{
@@ -105,6 +106,20 @@ int drm_colorop_init(struct drm_device *dev, struct
drm_colorop *colorop,
colorop->curve_1d_type_property,
0);
+ /* data */
+ if (type == DRM_COLOROP_CTM_3X4) {
+ prop = drm_property_create(dev, DRM_MODE_PROP_ATOMIC |
DRM_MODE_PROP_BLOB,
+ "DATA", 0);
+ if (!prop)
+ return -ENOMEM;
+
+ colorop->data_property = prop;
+ drm_object_attach_property(&colorop->base,
+ colorop->data_property,
+ 0);
+ }
+
+ /* next */
prop = drm_property_create_object(dev, DRM_MODE_PROP_IMMUTABLE |
DRM_MODE_PROP_ATOMIC,
"NEXT", DRM_MODE_OBJECT_COLOROP);
if (!prop)
@@ -219,6 +234,7 @@ EXPORT_SYMBOL(drm_colorop_reset);
static const char * const colorop_type_name[] = {
[DRM_COLOROP_1D_CURVE] = "1D Curve",
+ [DRM_COLOROP_CTM_3X4] = "3x4 Matrix"
};
static const char * const colorop_curve_1d_type_name[] = {
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 13acc9a6ac38..faca6eba10e1 100644
--- a/include/
[RFC PATCH v3 15/23] drm/vkms: Add enumerated 1D curve colorop
This patch introduces a VKMS color pipeline that includes two
drm_colorops for named transfer functions. For now the only ones
supported are sRGB EOTF, sRGB Inverse EOTF, and a Linear TF.
We will expand this in the future but I don't want to do so
without accompanying IGT tests.
We introduce a new vkms_luts.c file that hard-codes sRGB EOTF,
sRGB Inverse EOTF, and a linear EOTF LUT. These have been
generated with 256 entries each as IGT is currently testing
only 8 bpc surfaces. We will likely need higher precision
but I'm reluctant to make that change without clear indication
that we need it. We'll revisit and, if necessary, regenerate
the LUTs when we have IGT tests for higher precision buffers.
v2:
- Add commit description
- Fix sRGB EOTF LUT definition
- Add linear and sRGB inverse EOTF LUTs
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/vkms/Makefile| 4 +-
drivers/gpu/drm/vkms/vkms_colorop.c | 85 +++
drivers/gpu/drm/vkms/vkms_composer.c | 45 ++
drivers/gpu/drm/vkms/vkms_drv.h | 4 +
drivers/gpu/drm/vkms/vkms_luts.c | 802 +++
drivers/gpu/drm/vkms/vkms_luts.h | 12 +
drivers/gpu/drm/vkms/vkms_plane.c| 2 +
7 files changed, 953 insertions(+), 1 deletion(-)
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
diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
index 1b28a6a32948..c38455c46be4 100644
--- a/drivers/gpu/drm/vkms/Makefile
+++ b/drivers/gpu/drm/vkms/Makefile
@@ -6,6 +6,8 @@ vkms-y := \
vkms_formats.o \
vkms_crtc.o \
vkms_composer.o \
- vkms_writeback.o
+ vkms_writeback.o \
+ vkms_colorop.o \
+ vkms_luts.o
obj-$(CONFIG_DRM_VKMS) += vkms.o
diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c
b/drivers/gpu/drm/vkms/vkms_colorop.c
new file mode 100644
index ..9a26b9fdc4a2
--- /dev/null
+++ b/drivers/gpu/drm/vkms/vkms_colorop.c
@@ -0,0 +1,85 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_COLOR_PIPELINES 5
+
+const int vkms_initialize_tf_pipeline(struct drm_plane *plane, struct
drm_prop_enum_list *list)
+{
+
+ struct drm_colorop *op, *prev_op;
+ struct drm_device *dev = plane->dev;
+ int ret;
+
+ /* 1st op: 1d curve */
+ op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
+ if (!op) {
+ DRM_ERROR("KMS: Failed to allocate colorop\n");
+ return -ENOMEM;
+ }
+
+ ret = drm_colorop_init(dev, op, plane, DRM_COLOROP_1D_CURVE);
+ if (ret)
+ return ret;
+
+ list->type = op->base.id;
+ list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", op->base.id);
+
+ prev_op = op;
+
+ /* 2nd op: 1d curve */
+ op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
+ if (!op) {
+ DRM_ERROR("KMS: Failed to allocate colorop\n");
+ return -ENOMEM;
+ }
+
+ ret = drm_colorop_init(dev, op, plane, DRM_COLOROP_1D_CURVE);
+ if (ret)
+ return ret;
+
+ drm_colorop_set_next_property(prev_op, op);
+
+ return 0;
+}
+
+int vkms_initialize_colorops(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_property *prop;
+ struct drm_prop_enum_list pipelines[MAX_COLOR_PIPELINES];
+ int len = 0;
+ int ret;
+
+ /* Add "Bypass" (i.e. NULL) pipeline */
+ pipelines[len].type = 0;
+ pipelines[len].name = "Bypass";
+ len++;
+
+ /* Add pipeline consisting of transfer functions */
+ ret = vkms_initialize_tf_pipeline(plane, &(pipelines[len]));
+ if (ret)
+ return ret;
+ len++;
+
+ /* Create COLOR_PIPELINE property and attach */
+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
+ "COLOR_PIPELINE",
+ pipelines, len);
+ if (!prop)
+ return -ENOMEM;
+
+ plane->color_pipeline_property = prop;
+
+ drm_object_attach_property(&plane->base, prop, 0);
+
+ /* TODO do we even need this? */
+ if (plane->state)
+ plane->state->color_pipeline = NULL;
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 25b6b73bece8..be42756e300a 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -12,6 +12,7 @@
#include
#include "vkms_drv.h"
+#include "vkms_luts.h"
static u16 pre_mul_blend_channel(u16 src, u16 dst, u16 alpha)
{
@@ -163,6 +164,47 @@ static void apply_lut(const struct vkms_crtc_state
*crtc_state, struct line_buff
}
}
+static void pre_blend_color_transform(const struct vkms_plane_state
*plane_state, struct line_buffer *output_buffer)
+{
+
[RFC PATCH v3 22/23] drm/tests: Add a few tests around drm_fixed.h
While working on the CTM implementation of VKMS I had to ascertain
myself of a few assumptions. One of those is whether drm_fixed.h
treats its numbers using signed-magnitude or twos-complement. It is
twos-complement.
In order to make someone else's day easier I am adding the
drm_test_int2fixp test that validates the above assumption.
I am also adding a test for the new sm2fixp function that converts
from a signed-magnitude fixed point to the twos-complement fixed
point.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/tests/Makefile| 3 +-
drivers/gpu/drm/tests/drm_fixp_test.c | 69 +++
2 files changed, 71 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c
diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
index ba7baa622675..61f44ad0e862 100644
--- a/drivers/gpu/drm/tests/Makefile
+++ b/drivers/gpu/drm/tests/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
drm_plane_helper_test.o \
drm_probe_helper_test.o \
drm_rect_test.o \
- drm_exec_test.o
+ drm_exec_test.o \
+ drm_fixp_test.o
CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN)
diff --git a/drivers/gpu/drm/tests/drm_fixp_test.c
b/drivers/gpu/drm/tests/drm_fixp_test.c
new file mode 100644
index ..f420f173ff66
--- /dev/null
+++ b/drivers/gpu/drm/tests/drm_fixp_test.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2022 Advanced Micro Devices, Inc.
+ */
+
+#include
+#include
+
+static void drm_test_sm2fixp(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, 0x7fffll, ((1LL << 63) - 1));
+
+ /* 1 */
+ KUNIT_EXPECT_EQ(test, drm_int2fixp(1), drm_sm2fixp(1ull <<
DRM_FIXED_POINT));
+
+ /* -1 */
+ KUNIT_EXPECT_EQ(test, drm_int2fixp(-1), drm_sm2fixp((1ull << 63) |
(1ull << DRM_FIXED_POINT)));
+
+ /* 0.5 */
+ KUNIT_EXPECT_EQ(test, drm_fixp_from_fraction(1, 2), drm_sm2fixp(1ull <<
(DRM_FIXED_POINT - 1)));
+
+ /* -0.5 */
+ KUNIT_EXPECT_EQ(test, drm_fixp_from_fraction(-1, 2), drm_sm2fixp((1ull
<< 63) | (1ull << (DRM_FIXED_POINT - 1;
+
+}
+
+static void drm_test_int2fixp(struct kunit *test)
+{
+ /* 1 */
+ KUNIT_EXPECT_EQ(test, 1ll << 32, drm_int2fixp(1));
+
+ /* -1 */
+ KUNIT_EXPECT_EQ(test, -(1ll << 32), drm_int2fixp(-1));
+
+ /* 1 + (-1) = 0 */
+ KUNIT_EXPECT_EQ(test, 0, drm_int2fixp(1) + drm_int2fixp(-1));
+
+ /* 1 / 2 */
+ KUNIT_EXPECT_EQ(test, 1ll << 31, drm_fixp_from_fraction(1, 2));
+
+ /* -0.5 */
+ KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(-1, 2));
+
+ /* (1 / 2) + (-1) = 0.5 */
+ KUNIT_EXPECT_EQ(test, 1ll << 31, drm_fixp_from_fraction(-1, 2) +
drm_int2fixp(1));
+
+ /* (1 / 2) - 1) = 0.5 */
+ KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(1, 2) +
drm_int2fixp(-1));
+
+ /* (1 / 2) - 1) = 0.5 */
+ KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(1, 2) -
drm_int2fixp(1));
+
+}
+
+static struct kunit_case drm_fixp_tests[] = {
+ KUNIT_CASE(drm_test_int2fixp),
+ KUNIT_CASE(drm_test_sm2fixp),
+ { }
+};
+
+static struct kunit_suite drm_rect_test_suite = {
+ .name = "drm_fixp",
+ .test_cases = drm_fixp_tests,
+};
+
+kunit_test_suite(drm_rect_test_suite);
+
+MODULE_AUTHOR("AMD");
+MODULE_LICENSE("GPL and additional rights");
\ No newline at end of file
--
2.42.1
[RFC PATCH v3 20/23] drm/vkms: Use s32 for internal color pipeline precision
Certain operations require us to preserve values below 0.0 and
above 1.0 (0x0 and 0x respectively in 16 bpc unorm). One
such operation is a BT709 encoding operation followed by its
decoding operation, or the reverse.
We'll use s32 values as intermediate in and outputs of our
color operations, for the operations where it matters.
For now this won't apply to LUT operations. We might want to
update those to work on s32 as well, but it's unclear how
that should work for unorm LUT definitions. We'll revisit
that once we add LUT + CTM tests.
In order to allow for this we'll also invert the nesting of our
colorop processing loops. We now use the pixel iteration loop
on the outside and the colorop iteration on the inside.
v3:
- Use new colorop->next pointer
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/vkms/vkms_composer.c | 55 +---
drivers/gpu/drm/vkms/vkms_drv.h | 4 ++
2 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index 9010415e4bd6..d04a235b9fcd 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -164,7 +164,7 @@ static void apply_lut(const struct vkms_crtc_state
*crtc_state, struct line_buff
}
}
-static void apply_colorop(struct pixel_argb_u16 *pixel, struct drm_colorop
*colorop)
+static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop
*colorop)
{
/* TODO is this right? */
struct drm_colorop_state *colorop_state = colorop->state;
@@ -191,25 +191,54 @@ static void apply_colorop(struct pixel_argb_u16 *pixel,
struct drm_colorop *colo
static void pre_blend_color_transform(const struct vkms_plane_state
*plane_state, struct line_buffer *output_buffer)
{
- struct drm_colorop *colorop = plane_state->base.base.color_pipeline;
+ struct drm_colorop *colorop;
+ struct pixel_argb_s32 pixel;
- while (colorop) {
- struct drm_colorop_state *colorop_state;
+ for (size_t x = 0; x < output_buffer->n_pixels; x++) {
+
+ /*
+* Some operations, such as applying a BT709 encoding matrix,
+* followed by a decoding matrix, require that we preserve
+* values above 1.0 and below 0.0 until the end of the pipeline.
+*
+* Convert values to s32 for our internal pipeline and go back
+* to u16 values at the end.
+*/
+ pixel.a = output_buffer->pixels[x].a;
+ pixel.r = output_buffer->pixels[x].r;
+ pixel.g = output_buffer->pixels[x].g;
+ pixel.b = output_buffer->pixels[x].b;
+
+ colorop = plane_state->base.base.color_pipeline;
+ while (colorop) {
+ struct drm_colorop_state *colorop_state;
- if (!colorop)
- return;
+ if (!colorop)
+ return;
- /* TODO this is probably wrong */
- colorop_state = colorop->state;
+ /* TODO this is probably wrong */
+ colorop_state = colorop->state;
- if (!colorop_state)
- return;
+ if (!colorop_state)
+ return;
- for (size_t x = 0; x < output_buffer->n_pixels; x++)
if (!colorop_state->bypass)
- apply_colorop(&output_buffer->pixels[x],
colorop);
+ apply_colorop(&pixel, colorop);
+
+ colorop = colorop->next;
+ }
- colorop = colorop->next;
+ /* clamp pixel */
+ pixel.a = max(min(pixel.a, 0x), 0x0);
+ pixel.r = max(min(pixel.r, 0x), 0x0);
+ pixel.g = max(min(pixel.g, 0x), 0x0);
+ pixel.b = max(min(pixel.b, 0x), 0x0);
+
+ /* put back to output_buffer */
+ output_buffer->pixels[x].a = pixel.a;
+ output_buffer->pixels[x].r = pixel.r;
+ output_buffer->pixels[x].g = pixel.g;
+ output_buffer->pixels[x].b = pixel.b;
}
}
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 2bcc24c196a2..fadb7685a360 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -36,6 +36,10 @@ struct vkms_frame_info {
unsigned int cpp;
};
+struct pixel_argb_s32 {
+ s32 a, r, g, b;
+};
+
struct pixel_argb_u16 {
u16 a, r, g, b;
};
--
2.42.1
[RFC PATCH v3 21/23] drm/vkms: add 3x4 matrix in color pipeline
We add two 3x4 matrices into the VKMS color pipeline. The reason
we're adding matrices is so that we can test that application
of a matrix and its inverse yields an output equal to the input
image.
One complication with the matrix implementation has to do with
the fact that the matrix entries are in signed-magnitude fixed
point, whereas the drm_fixed.h implementation uses 2s-complement.
The latter one is the one that we want for easy addition and
subtraction, so we convert all entries to 2s-complement.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/vkms/vkms_colorop.c | 32 +++-
drivers/gpu/drm/vkms/vkms_composer.c | 27 +++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c
b/drivers/gpu/drm/vkms/vkms_colorop.c
index 9a26b9fdc4a2..4e37e805c443 100644
--- a/drivers/gpu/drm/vkms/vkms_colorop.c
+++ b/drivers/gpu/drm/vkms/vkms_colorop.c
@@ -31,7 +31,37 @@ const int vkms_initialize_tf_pipeline(struct drm_plane
*plane, struct drm_prop_e
prev_op = op;
- /* 2nd op: 1d curve */
+ /* 2nd op: 3x4 matrix */
+ op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
+ if (!op) {
+ DRM_ERROR("KMS: Failed to allocate colorop\n");
+ return -ENOMEM;
+ }
+
+ ret = drm_colorop_init(dev, op, plane, DRM_COLOROP_CTM_3X4);
+ if (ret)
+ return ret;
+
+ drm_colorop_set_next_property(prev_op, op);
+
+ prev_op = op;
+
+ /* 3rd op: 3x4 matrix */
+ op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
+ if (!op) {
+ DRM_ERROR("KMS: Failed to allocate colorop\n");
+ return -ENOMEM;
+ }
+
+ ret = drm_colorop_init(dev, op, plane, DRM_COLOROP_CTM_3X4);
+ if (ret)
+ return ret;
+
+ drm_colorop_set_next_property(prev_op, op);
+
+ prev_op = op;
+
+ /* 4th op: 1d curve */
op = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
if (!op) {
DRM_ERROR("KMS: Failed to allocate colorop\n");
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index d04a235b9fcd..c278fb223188 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -164,6 +164,30 @@ static void apply_lut(const struct vkms_crtc_state
*crtc_state, struct line_buff
}
}
+static void apply_3x4_matrix(struct pixel_argb_s32 *pixel, const struct
drm_color_ctm_3x4 *matrix)
+{
+ s64 rf, gf, bf;
+
+ rf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[0]),
drm_int2fixp(pixel->r)) +
+drm_fixp_mul(drm_sm2fixp(matrix->matrix[1]),
drm_int2fixp(pixel->g)) +
+drm_fixp_mul(drm_sm2fixp(matrix->matrix[2]),
drm_int2fixp(pixel->b)) +
+drm_sm2fixp(matrix->matrix[3]);
+
+ gf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[4]),
drm_int2fixp(pixel->r)) +
+drm_fixp_mul(drm_sm2fixp(matrix->matrix[5]),
drm_int2fixp(pixel->g)) +
+drm_fixp_mul(drm_sm2fixp(matrix->matrix[6]),
drm_int2fixp(pixel->b)) +
+drm_sm2fixp(matrix->matrix[7]);
+
+ bf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[8]),
drm_int2fixp(pixel->r)) +
+drm_fixp_mul(drm_sm2fixp(matrix->matrix[9]),
drm_int2fixp(pixel->g)) +
+drm_fixp_mul(drm_sm2fixp(matrix->matrix[10]),
drm_int2fixp(pixel->b)) +
+drm_sm2fixp(matrix->matrix[11]);
+
+ pixel->r = drm_fixp2int(rf);
+ pixel->g = drm_fixp2int(gf);
+ pixel->b = drm_fixp2int(bf);
+}
+
static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop
*colorop)
{
/* TODO is this right? */
@@ -185,6 +209,9 @@ static void apply_colorop(struct pixel_argb_s32 *pixel,
struct drm_colorop *colo
DRM_DEBUG_DRIVER("unkown colorop 1D curve type
%d\n", colorop_state->curve_1d_type);
break;
}
+ } else if (colorop->type == DRM_COLOROP_CTM_3X4) {
+ if (colorop_state->data)
+ apply_3x4_matrix(pixel, (struct drm_color_ctm_3x4 *)
colorop_state->data->data);
}
}
--
2.42.1
[RFC PATCH v3 23/23] drm/vkms: Add tests for CTM handling
A whole slew of tests for CTM handling that greatly helped in
debugging the CTM code. The extent of tests might seem a bit
silly but they're fast and might someday help save someone
else's day when debugging this.
Signed-off-by: Harry Wentland
---
drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 258 ++
drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
2 files changed, 259 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
index ad4c2f72fd1e..3eaa2233afbb 100644
--- a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
+++ b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
@@ -3,6 +3,7 @@
#include
#include
+#include
#define TEST_LUT_SIZE 16
@@ -80,11 +81,268 @@ static void vkms_color_srgb_inv_srgb(struct kunit *test)
}
}
+#define FIXPT_HALF(DRM_FIXED_ONE >> 1)
+#define FIXPT_QUARTER (DRM_FIXED_ONE >> 2)
+
+const struct drm_color_ctm_3x4 test_matrix_3x4_50_desat = { {
+ FIXPT_HALF, FIXPT_QUARTER, FIXPT_QUARTER, 0,
+ FIXPT_QUARTER, FIXPT_HALF, FIXPT_QUARTER, 0,
+ FIXPT_QUARTER, FIXPT_QUARTER, FIXPT_HALF, 0
+} };
+
+static void vkms_color_ctm_3x4_50_desat(struct kunit *test)
+{
+ struct pixel_argb_s32 ref, out;
+
+ /* full white */
+ ref.a = 0x0;
+ ref.r = 0x;
+ ref.g = 0x;
+ ref.b = 0x;
+
+ memcpy(&out, &ref, sizeof(out));
+ apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
+
+ KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
+
+ /* full black */
+ ref.a = 0x0;
+ ref.r = 0x0;
+ ref.g = 0x0;
+ ref.b = 0x0;
+
+ memcpy(&out, &ref, sizeof(out));
+ apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
+
+ KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
+
+ /* 50% grey */
+ ref.a = 0x0;
+ ref.r = 0x8000;
+ ref.g = 0x8000;
+ ref.b = 0x8000;
+
+ memcpy(&out, &ref, sizeof(out));
+ apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
+
+ KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
+
+ /* full red to 50% desat */
+ ref.a = 0x0;
+ ref.r = 0x7fff;
+ ref.g = 0x3fff;
+ ref.b = 0x3fff;
+
+ out.a = 0x0;
+ out.r = 0x;
+ out.g = 0x0;
+ out.b = 0x0;
+
+ apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
+
+ KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
+}
+
+const struct drm_color_ctm_3x4 test_matrix_3x4_bt709_enc = { {
+ 0x366cf400ull, 0xb7175900ull, 0x000127bb300ull, 0,
+ 0x80001993b3a0ull, 0x80005609fe80ull, 0x6f9db200ull, 0,
+ 0x9d70a400ull, 0x80008f011100ull, 0x8e6f9330ull, 0
+} };
+
+static void vkms_color_ctm_3x4_bt709(struct kunit *test)
+{
+ struct pixel_argb_s32 ref, out;
+
+ /* full white to bt709 */
+ ref.a = 0x0;
+ ref.r = 0xfffe; /* off by one in 16bpc not a big deal */
+ ref.g = 0x0;
+ ref.b = 0x0;
+
+ out.a = 0x0;
+ out.r = 0x;
+ out.g = 0x;
+ out.b = 0x;
+
+ apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
+
+ /* red 255 */
+ KUNIT_EXPECT_GT(test, out.r, 0xfe00);
+
+ /* green 0 */
+ KUNIT_EXPECT_LT(test, out.g, 0x0100);
+
+ /* blue 0 */
+ KUNIT_EXPECT_LT(test, out.b, 0x0100);
+
+ /* full black to bt709 */
+ ref.a = 0x0;
+ ref.r = 0x0; /* off by one in 16bpc not a big deal */
+ ref.g = 0x0;
+ ref.b = 0x0;
+
+ out.a = 0x0;
+ out.r = 0x0;
+ out.g = 0x0;
+ out.b = 0x0;
+
+ apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
+
+ /* red 0 */
+ KUNIT_EXPECT_LT(test, out.r, 0x100);
+
+ /* green 0 */
+ KUNIT_EXPECT_LT(test, out.g, 0x0100);
+
+ /* blue 0 */
+ KUNIT_EXPECT_LT(test, out.b, 0x0100);
+
+ /* gray to bt709 */
+ ref.a = 0x0;
+ ref.r = 0x7fff; /* off by one in 16bpc not a big deal */
+ ref.g = 0x0;
+ ref.b = 0x0;
+
+ out.a = 0x0;
+ out.r = 0x7fff;
+ out.g = 0x7fff;
+ out.b = 0x7fff;
+
+ apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
+
+ /* red 127 */
+ KUNIT_EXPECT_GT(test, out.r, 0x7e00);
+ KUNIT_EXPECT_LT(test, out.r, 0x8000);
+
+ /* green 0 */
+ KUNIT_EXPECT_LT(test, out.g, 0x0100);
+
+ /* blue 0 */
+ KUNIT_EXPECT_LT(test, out.b, 0x0100);
+
+ /* == red 255 - bt709 enc == */
+ out.a = 0x0;
+ out.r = 0x;
+ out.g = 0x0;
+ out.b = 0x0;
+
+ apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
+
+ /* red 54 */
+ KUNIT_EXPECT_GT(test, out.r, 0x3500);
+ KUNIT_EXPECT_LT(test, out.r, 0x3700);
+
+ /* green 0 */
+ KUNIT_EXPECT_LT(test, out.g, 0x0100);
+
+ /* blue 157 */
+ KUNIT_EXPECT_GT(test, out.b, 0x9C00);
+ KUNIT_EXPECT_LT(test, out.b, 0x9E00);
+
+
+
