| Message ID | 20221212182137.374625-5-harry.wentland@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable Colorspace connector property in amdgpu | expand |
On Mon, 12 Dec 2022 13:21:25 -0500 Harry Wentland <harry.wentland@amd.com> wrote: > This allows us to use strongly typed arguments. > > Signed-off-by: Harry Wentland <harry.wentland@amd.com> > Cc: Pekka Paalanen <ppaalanen@gmail.com> > Cc: Sebastian Wick <sebastian.wick@redhat.com> > Cc: Vitaly.Prosyak@amd.com > Cc: Uma Shankar <uma.shankar@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Joshua Ashton <joshua@froggi.es> > Cc: dri-devel@lists.freedesktop.org > Cc: amd-gfx@lists.freedesktop.org > --- > include/drm/display/drm_dp.h | 2 +- > include/drm/drm_connector.h | 47 ++++++++++++++++++------------------ > 2 files changed, 25 insertions(+), 24 deletions(-) > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 4d0abe4c7ea9..b98697459f9c 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -1615,7 +1615,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 62c814241828..edef65388c29 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -371,28 +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_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, > +}; Hi, what's the entry for "the traditional sRGB"? It cannot be DRM_MODE_COLORIMETRY_DEFAULT, because the doc here says that it maps to some driver-defined entry which may be something else. Thanks, pq > > /** > * enum drm_bus_flags - bus_flags info for &drm_display_info > @@ -825,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
On 12/13/22 05:39, Pekka Paalanen wrote: > On Mon, 12 Dec 2022 13:21:25 -0500 > Harry Wentland <harry.wentland@amd.com> wrote: > >> This allows us to use strongly typed arguments. >> >> Signed-off-by: Harry Wentland <harry.wentland@amd.com> >> Cc: Pekka Paalanen <ppaalanen@gmail.com> >> Cc: Sebastian Wick <sebastian.wick@redhat.com> >> Cc: Vitaly.Prosyak@amd.com >> Cc: Uma Shankar <uma.shankar@intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Cc: Joshua Ashton <joshua@froggi.es> >> Cc: dri-devel@lists.freedesktop.org >> Cc: amd-gfx@lists.freedesktop.org >> --- >> include/drm/display/drm_dp.h | 2 +- >> include/drm/drm_connector.h | 47 ++++++++++++++++++------------------ >> 2 files changed, 25 insertions(+), 24 deletions(-) >> >> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h >> index 4d0abe4c7ea9..b98697459f9c 100644 >> --- a/include/drm/display/drm_dp.h >> +++ b/include/drm/display/drm_dp.h >> @@ -1615,7 +1615,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 62c814241828..edef65388c29 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -371,28 +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_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, >> +}; > > Hi, > > what's the entry for "the traditional sRGB"? > > It cannot be DRM_MODE_COLORIMETRY_DEFAULT, because the doc here says > that it maps to some driver-defined entry which may be something else. > If I understand this list correctly the only entry that currently covers sRGB is DEFAULT. Harry > > Thanks, > pq > >> >> /** >> * enum drm_bus_flags - bus_flags info for &drm_display_info >> @@ -825,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 >
On Tue, 13 Dec 2022 11:41:08 -0500 Harry Wentland <harry.wentland@amd.com> wrote: > On 12/13/22 05:39, Pekka Paalanen wrote: > > On Mon, 12 Dec 2022 13:21:25 -0500 > > Harry Wentland <harry.wentland@amd.com> wrote: > > > >> This allows us to use strongly typed arguments. > >> > >> Signed-off-by: Harry Wentland <harry.wentland@amd.com> > >> Cc: Pekka Paalanen <ppaalanen@gmail.com> > >> Cc: Sebastian Wick <sebastian.wick@redhat.com> > >> Cc: Vitaly.Prosyak@amd.com > >> Cc: Uma Shankar <uma.shankar@intel.com> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Cc: Joshua Ashton <joshua@froggi.es> > >> Cc: dri-devel@lists.freedesktop.org > >> Cc: amd-gfx@lists.freedesktop.org > >> --- > >> include/drm/display/drm_dp.h | 2 +- > >> include/drm/drm_connector.h | 47 ++++++++++++++++++------------------ > >> 2 files changed, 25 insertions(+), 24 deletions(-) > >> > >> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > >> index 4d0abe4c7ea9..b98697459f9c 100644 > >> --- a/include/drm/display/drm_dp.h > >> +++ b/include/drm/display/drm_dp.h > >> @@ -1615,7 +1615,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 62c814241828..edef65388c29 100644 > >> --- a/include/drm/drm_connector.h > >> +++ b/include/drm/drm_connector.h > >> @@ -371,28 +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_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, > >> +}; > > > > Hi, > > > > what's the entry for "the traditional sRGB"? > > > > It cannot be DRM_MODE_COLORIMETRY_DEFAULT, because the doc here says > > that it maps to some driver-defined entry which may be something else. > > > > If I understand this list correctly the only entry that currently covers > sRGB is DEFAULT. Then either the list or the doc needs fixing: - If DEFAULT is driver-chosen, we really do need an entry for "the traditional (s)RGB", which corresponds to the bits Y0, Y1, Y2, C0, C1 being all zeros (CTA-861-H). - If DEFAULT means that the bits Y0, Y1, Y2, C0, C1 are all zeros, then it's not driver-chosen, and the doc needs to be more clear about this. But since there are really the UAPI (right?), this should be said in the UAPI docs for "Colorspace" property, and the internal doc here could just point to that. I think this could be just misdocumentation, because in CTA-861-H, the state Y0=0, Y1=0, Y2=0 is labeled as "RGB (default)" in table Table 16 - AVI InfoFrame RGB or YC B C R Field, Data Byte 1. It's not "driver will set something", it's "whatever the sink expects when nothing is explicitly said otherwise" which I assume is practically "the traditional RGB". C0=0, C1=0 means "no colorimetry data". This is something we need to be able to explicitly choose in userspace, when that is what we want to have. Thanks, pq
diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 4d0abe4c7ea9..b98697459f9c 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -1615,7 +1615,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 62c814241828..edef65388c29 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -371,28 +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_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 @@ -825,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
This allows us to use strongly typed arguments. Signed-off-by: Harry Wentland <harry.wentland@amd.com> Cc: Pekka Paalanen <ppaalanen@gmail.com> Cc: Sebastian Wick <sebastian.wick@redhat.com> Cc: Vitaly.Prosyak@amd.com Cc: Uma Shankar <uma.shankar@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Joshua Ashton <joshua@froggi.es> Cc: dri-devel@lists.freedesktop.org Cc: amd-gfx@lists.freedesktop.org --- include/drm/display/drm_dp.h | 2 +- include/drm/drm_connector.h | 47 ++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 24 deletions(-)