Re: [PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum
On Fri, 3 Feb 2023 02:07:42 + Joshua Ashton wrote: > From: Harry Wentland > > This allows us to use strongly typed arguments. > > Signed-off-by: Harry Wentland > Reviewed-by: Simon Ser > > Cc: Pekka Paalanen > Cc: Sebastian Wick > Cc: vitaly.pros...@amd.com > Cc: Uma Shankar > Cc: Ville Syrjälä > Cc: Joshua Ashton > Cc: dri-de...@lists.freedesktop.org > Cc: amd-gfx@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 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_6015 > -#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 _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; > +
Re: [PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum
Hi Joshua, I love your patch! Yet something to improve: [auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on drm/drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes linus/master v6.2-rc6 next-20230203] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Joshua-Ashton/drm-connector-Add-enum-documentation-to-drm_colorspace/20230203-100927 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230203020744.30745-1-joshua%40froggi.es patch subject: [PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20230204/202302041801.feskjrem-...@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/146e4b57c07c6b478f24bb7cf2603ab123dcb6f2 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Joshua-Ashton/drm-connector-Add-enum-documentation-to-drm_colorspace/20230203-100927 git checkout 146e4b57c07c6b478f24bb7cf2603ab123dcb6f2 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/display/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All errors (new ones prefixed by >>): >> drivers/gpu/drm/display/drm_hdmi_helper.c:108:3: error: use of undeclared >> identifier 'DRM_MODE_COLORIMETRY_NO_DATA'; did you mean >> 'DRM_MODE_COLORIMETRY_OPRGB'? [DRM_MODE_COLORIMETRY_NO_DATA] = HDMI_COLORIMETRY_NO_DATA, ^~~~ DRM_MODE_COLORIMETRY_OPRGB include/drm/drm_connector.h:441:2: note: 'DRM_MODE_COLORIMETRY_OPRGB' declared here DRM_MODE_COLORIMETRY_OPRGB, ^ drivers/gpu/drm/display/drm_hdmi_helper.c:115:33: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides] [DRM_MODE_COLORIMETRY_OPRGB] = HDMI_COLORIMETRY_OPRGB, ^~ drivers/gpu/drm/display/drm_hdmi_helper.c:100:34: note: expanded from macro 'HDMI_COLORIMETRY_OPRGB' #define HDMI_COLORIMETRY_OPRGB (C(3) | EC(4) | ACE(0)) ^~~ drivers/gpu/drm/display/drm_hdmi_helper.c:108:35: note: previous initialization is here [DRM_MODE_COLORIMETRY_NO_DATA] = HDMI_COLORIMETRY_NO_DATA, ^~~~ drivers/gpu/drm/display/drm_hdmi_helper.c:93:35: note: expanded from macro 'HDMI_COLORIMETRY_NO_DATA' #define HDMI_COLORIMETRY_NO_DATA0x0 ^~~ 1 warning and 1 error generated. vim +108 drivers/gpu/drm/display/drm_hdmi_helper.c 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 106 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 107 static const u32 hdmi_colorimetry_val[] = { 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 @108 [DRM_MODE_COLORIMETRY_NO_DATA] = HDMI_COLORIMETRY_NO_DATA, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 109 [DRM_MODE_COLORIMETRY_SMPTE_170M_YCC] = HDMI_COLORIMETRY_SMPTE_170M_YCC, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 110 [DRM_MODE_COLORIMETRY_BT709_YCC] = HDMI_COLORIMETRY_BT709_YCC, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 111 [DRM_MODE_COLORIMETRY_XVYCC_601] = HDMI_COLORIMETRY_XVYCC_601, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 112 [DRM_MODE_COLORIMETRY_XVYCC_709] = HDMI_COLORIMETRY_XVYCC_709, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 113 [DRM_MODE_COLORIMETRY_SYCC_601] = HDMI_COLORIMETRY_SYCC_601, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 114 [DRM_MODE_COLORIMETRY_OPYCC_601] = HDMI_COLORIMETRY_OPYCC_601, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 115 [DRM_MODE_COLORIMETRY_OPRGB] = HDMI_COLORIMETRY_OPRGB, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 116 [DRM_MODE_COLORIMETRY_BT2020_CYCC] = HDMI_COLORIMETRY_BT2020_CYCC, 4fc8cb47fcfdc93 Thomas Zimmermann 2022-04-21 117 [DRM_MODE_COLORIMETRY_BT2020_RGB] = HDMI_COLORIMETRY_BT2020_RGB, 4fc8cb47fcfd
Re: [PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum
Hi Joshua, I love your patch! Yet something to improve: [auto build test ERROR on drm-misc/drm-misc-next] [also build test ERROR on drm/drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes linus/master v6.2-rc6 next-20230203] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Joshua-Ashton/drm-connector-Add-enum-documentation-to-drm_colorspace/20230203-100927 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20230203020744.30745-1-joshua%40froggi.es patch subject: [PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum config: arc-randconfig-r034-20230204 (https://download.01.org/0day-ci/archive/20230204/202302041649.dswc7gnr-...@intel.com/config) compiler: arc-elf-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/146e4b57c07c6b478f24bb7cf2603ab123dcb6f2 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Joshua-Ashton/drm-connector-Add-enum-documentation-to-drm_colorspace/20230203-100927 git checkout 146e4b57c07c6b478f24bb7cf2603ab123dcb6f2 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/display/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All errors (new ones prefixed by >>): >> drivers/gpu/drm/display/drm_hdmi_helper.c:108:10: error: >> 'DRM_MODE_COLORIMETRY_NO_DATA' undeclared here (not in a function); did you >> mean 'DRM_MODE_COLORIMETRY_OPRGB'? 108 | [DRM_MODE_COLORIMETRY_NO_DATA] = HDMI_COLORIMETRY_NO_DATA, | ^~~~ | DRM_MODE_COLORIMETRY_OPRGB >> drivers/gpu/drm/display/drm_hdmi_helper.c:108:10: error: array index in >> initializer not of integer type drivers/gpu/drm/display/drm_hdmi_helper.c:108:10: note: (near initialization for 'hdmi_colorimetry_val') vim +108 drivers/gpu/drm/display/drm_hdmi_helper.c 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 106 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 107 static const u32 hdmi_colorimetry_val[] = { 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 @108 [DRM_MODE_COLORIMETRY_NO_DATA] = HDMI_COLORIMETRY_NO_DATA, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 109 [DRM_MODE_COLORIMETRY_SMPTE_170M_YCC] = HDMI_COLORIMETRY_SMPTE_170M_YCC, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 110 [DRM_MODE_COLORIMETRY_BT709_YCC] = HDMI_COLORIMETRY_BT709_YCC, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 111 [DRM_MODE_COLORIMETRY_XVYCC_601] = HDMI_COLORIMETRY_XVYCC_601, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 112 [DRM_MODE_COLORIMETRY_XVYCC_709] = HDMI_COLORIMETRY_XVYCC_709, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 113 [DRM_MODE_COLORIMETRY_SYCC_601] = HDMI_COLORIMETRY_SYCC_601, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 114 [DRM_MODE_COLORIMETRY_OPYCC_601] = HDMI_COLORIMETRY_OPYCC_601, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 115 [DRM_MODE_COLORIMETRY_OPRGB] = HDMI_COLORIMETRY_OPRGB, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 116 [DRM_MODE_COLORIMETRY_BT2020_CYCC] = HDMI_COLORIMETRY_BT2020_CYCC, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 117 [DRM_MODE_COLORIMETRY_BT2020_RGB] = HDMI_COLORIMETRY_BT2020_RGB, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 118 [DRM_MODE_COLORIMETRY_BT2020_YCC] = HDMI_COLORIMETRY_BT2020_YCC, 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 119 }; 4fc8cb47fcfdc9 Thomas Zimmermann 2022-04-21 120 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
[PATCH 1/3] drm/connector: Convert DRM_MODE_COLORIMETRY to enum
From: Harry Wentland This allows us to use strongly typed arguments. Signed-off-by: Harry Wentland Reviewed-by: Simon Ser Cc: Pekka Paalanen Cc: Sebastian Wick Cc: vitaly.pros...@amd.com Cc: Uma Shankar Cc: Ville Syrjälä Cc: Joshua Ashton Cc: dri-de...@lists.freedesktop.org Cc: amd-gfx@lists.freedesktop.org --- include/drm/display/drm_dp.h | 2 +- include/drm/drm_connector.h | 48 ++-- 2 files changed, 25 insertions(+), 25 deletions(-) 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 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_YCC1 -#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_RGB9 -#define DRM_MODE_COLORIMETRY_BT2020_YCC10 -/* Additional Colorimetry extension added as part of CTA 861.G */ -#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_D6511 -#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER12 -/* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry Format */ -#define DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED13 -#define DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT14 -#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 _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 -- 2.39.1