On Fri,  3 Feb 2023 02:07:42 +0000
Joshua Ashton <jos...@froggi.es> wrote:

> From: Harry Wentland <harry.wentl...@amd.com>
> 
> This allows us to use strongly typed arguments.
> 
> Signed-off-by: Harry Wentland <harry.wentl...@amd.com>
> Reviewed-by: Simon Ser <cont...@emersion.fr>
> 
> Cc: Pekka Paalanen <ppaala...@gmail.com>
> Cc: Sebastian Wick <sebastian.w...@redhat.com>
> Cc: vitaly.pros...@amd.com
> Cc: Uma Shankar <uma.shan...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Cc: Joshua Ashton <jos...@froggi.es>
> Cc: dri-devel@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> ---
>  include/drm/display/drm_dp.h |  2 +-
>  include/drm/drm_connector.h  | 48 ++++++++++++++++++------------------
>  2 files changed, 25 insertions(+), 25 deletions(-)
> 

Hi,

the code changes I can actually see here look good, but the test bot
found something else to fix. I feel the disappearance of
DRM_MODE_COLORIMETRY_NO_DATA could use an explanation in the commit
message.

I can only guess that NO_DATA comes from HDMI or DP spec or some such
to indicate undefined or something. However, the API here repurposes
that code point for "driver picks whatever".

I suppose it's kernel style to not write out the enum values when the C
standard rules produce the right values, but personally I think that is
hard to review and prone to accidental breakage if someone goes to add
a new value in the middle. Assuming these values are supposed to match
with a spec. I have no idea if they are.


Thanks,
pq

> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index ed10e6b6f99d..28899a03245c 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -1623,7 +1623,7 @@ enum dp_pixelformat {
>   *
>   * This enum is used to indicate DP VSC SDP Colorimetry formats.
>   * It is based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through
> - * DB18] and a name of enum member follows DRM_MODE_COLORIMETRY definition.
> + * DB18] and a name of enum member follows &enum drm_colorimetry definition.
>   *
>   * @DP_COLORIMETRY_DEFAULT: sRGB (IEC 61966-2-1) or
>   *                          ITU-R BT.601 colorimetry format
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4d830fc55a3d..edef65388c29 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -371,29 +371,29 @@ enum drm_privacy_screen_status {
>   * a colorspace property which will be created and exposed to
>   * userspace.
>   */
> -
> -/* For Default case, driver will set the colorspace */
> -#define DRM_MODE_COLORIMETRY_DEFAULT                 0
> -/* CEA 861 Normal Colorimetry options */
> -#define DRM_MODE_COLORIMETRY_NO_DATA                 0
> -#define DRM_MODE_COLORIMETRY_SMPTE_170M_YCC          1
> -#define DRM_MODE_COLORIMETRY_BT709_YCC                       2
> -/* CEA 861 Extended Colorimetry Options */
> -#define DRM_MODE_COLORIMETRY_XVYCC_601                       3
> -#define DRM_MODE_COLORIMETRY_XVYCC_709                       4
> -#define DRM_MODE_COLORIMETRY_SYCC_601                        5
> -#define DRM_MODE_COLORIMETRY_OPYCC_601                       6
> -#define DRM_MODE_COLORIMETRY_OPRGB                   7
> -#define DRM_MODE_COLORIMETRY_BT2020_CYCC             8
> -#define DRM_MODE_COLORIMETRY_BT2020_RGB                      9
> -#define DRM_MODE_COLORIMETRY_BT2020_YCC                      10
> -/* Additional Colorimetry extension added as part of CTA 861.G */
> -#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65          11
> -#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER              12
> -/* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry Format */
> -#define DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED          13
> -#define DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT          14
> -#define DRM_MODE_COLORIMETRY_BT601_YCC                       15
> +enum drm_colorspace {
> +     /* For Default case, driver will set the colorspace */
> +     DRM_MODE_COLORIMETRY_DEFAULT,
> +     /* CEA 861 Normal Colorimetry options */
> +     DRM_MODE_COLORIMETRY_SMPTE_170M_YCC,
> +     DRM_MODE_COLORIMETRY_BT709_YCC,
> +     /* CEA 861 Extended Colorimetry Options */
> +     DRM_MODE_COLORIMETRY_XVYCC_601,
> +     DRM_MODE_COLORIMETRY_XVYCC_709,
> +     DRM_MODE_COLORIMETRY_SYCC_601,
> +     DRM_MODE_COLORIMETRY_OPYCC_601,
> +     DRM_MODE_COLORIMETRY_OPRGB,
> +     DRM_MODE_COLORIMETRY_BT2020_CYCC,
> +     DRM_MODE_COLORIMETRY_BT2020_RGB,
> +     DRM_MODE_COLORIMETRY_BT2020_YCC,
> +     /* Additional Colorimetry extension added as part of CTA 861.G */
> +     DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65,
> +     DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER,
> +     /* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry 
> Format */
> +     DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED,
> +     DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT,
> +     DRM_MODE_COLORIMETRY_BT601_YCC,
> +};
>  
>  /**
>   * enum drm_bus_flags - bus_flags info for &drm_display_info
> @@ -826,7 +826,7 @@ struct drm_connector_state {
>        * colorspace change on Sink. This is most commonly used to switch
>        * to wider color gamuts like BT2020.
>        */
> -     u32 colorspace;
> +     enum drm_colorspace colorspace;
>  
>       /**
>        * @writeback_job: Writeback job for writeback connectors

Attachment: pgptHSCOI0Z6W.pgp
Description: OpenPGP digital signature

Reply via email to