diff mbox series

[v3,01/17] drm: add DP 1.4 VSC SDP Payload related enums and a structure

Message ID 20200203232014.906651-2-gwan-gyeong.mun@intel.com (mailing list archive)
State New, archived
Headers show
Series In order to readout DP SDPs, refactors the handling of DP SDPs | expand

Commit Message

Gwan-gyeong Mun Feb. 3, 2020, 11:19 p.m. UTC 
It adds new enumeration definitions for VSC SDP Payload for Pixel
Encoding/Colorimetry Format.
And it adds a new drm data structure for DP VSC SDP.

enum dp_colorspace and enum dp_colorimetry correspond "Pixel Encoding and
Colorimetry Formats". enum dp_dynamic_range corresponds "Dynamic Range".
And enum dp_content_type corresponds "Content Type"
All of them are based on DP 1.4 spec [Table 2-117: VSC SDP Payload for
DB16 through DB18].

v3: Add a new drm data structure for DP VSC SDP

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 include/drm/drm_dp_helper.h | 57 +++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

Comments

Shankar, Uma Feb. 5, 2020, 2:42 p.m. UTC | #1 
> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Gwan-
> gyeong Mun
> Sent: Tuesday, February 4, 2020 4:50 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: linux-fbdev@vger.kernel.org; dri-devel@lists.freedesktop.org
> Subject: [PATCH v3 01/17] drm: add DP 1.4 VSC SDP Payload related enums and a
> structure

%s/add/Add/
Also you can rephrase this as " Add DP1.4 VSC SDP Payload related Data Structures"/

> It adds new enumeration definitions for VSC SDP Payload for Pixel
> Encoding/Colorimetry Format.
> And it adds a new drm data structure for DP VSC SDP.
> 
> enum dp_colorspace and enum dp_colorimetry correspond "Pixel Encoding and
> Colorimetry Formats". enum dp_dynamic_range corresponds "Dynamic Range".
> And enum dp_content_type corresponds "Content Type"
> All of them are based on DP 1.4 spec [Table 2-117: VSC SDP Payload for
> DB16 through DB18].
> 
> v3: Add a new drm data structure for DP VSC SDP
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  include/drm/drm_dp_helper.h | 57 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index
> 262faf9e5e94..c098727681fa 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1209,6 +1209,63 @@ struct dp_sdp {
>  #define EDP_VSC_PSR_UPDATE_RFB		(1<<1)
>  #define EDP_VSC_PSR_CRC_VALUES_VALID	(1<<2)
> 
> +/* Based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through
> +DB18] */ enum dp_colorspace {

We should not call this colorspace, rather rename it to dp_pixelformat.
These are not colorspaces.

Also add these to kernel docs as they are standard definitions.

> +	DP_COLORSPACE_RGB = 0,

Make this as DP_PIXELFORMAT_RGB and rename all the below ones as well.

> +	DP_COLORSPACE_YUV444 = 0x1,
> +	DP_COLORSPACE_YUV422 = 0x2,
> +	DP_COLORSPACE_YUV420 = 0x3,
> +	DP_COLORSPACE_Y_ONLY = 0x4,
> +	DP_COLORSPACE_RAW = 0x5,
> +	DP_COLORSPACE_RESERVED = 0x6,
> +};
> +
> +/**
> + * Based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through
> +DB18]
> + * and a name of enum member followes DRM_MODE_COLORIMETRY definition.

Typo in follows

> + */
> +enum dp_colorimetry {

You can call this as dp_colorspace (this is actual colorspace), you can stick with colorimetry as well.
Will leave that to you.

> +	DP_COLORIMETRY_DEFAULT = 0, /* sRGB (IEC 61966-2-1) / ITU-R BT.601 */
> +	DP_COLORIMETRY_RGB_WIDE_FIXED = 0x1,
> +	DP_COLORIMETRY_BT709_YCC = 0x1,
> +	DP_COLORIMETRY_RGB_WIDE_FLOAT = 0x2,
> +	DP_COLORIMETRY_XVYCC_601 = 0x2,
> +	DP_COLORIMETRY_OPRGB = 0x3,
> +	DP_COLORIMETRY_XVYCC_709 = 0x3,
> +	DP_COLORIMETRY_DCI_P3_RGB = 0x4,
> +	DP_COLORIMETRY_SYCC_601 = 0x4,
> +	DP_COLORIMETRY_RGB_CUSTOM = 0x5,
> +	DP_COLORIMETRY_OPYCC_601 = 0x5,
> +	DP_COLORIMETRY_BT2020_RGB = 0x6,
> +	DP_COLORIMETRY_BT2020_CYCC = 0x6,
> +	DP_COLORIMETRY_BT2020_YCC = 0x7,
> +};
> +
> +enum dp_dynamic_range {
> +	DP_DYNAMIC_RANGE_VESA = 0,
> +	DP_DYNAMIC_RANGE_CTA = 1,
> +};
> +
> +enum dp_content_type {
> +	DP_CONTENT_TYPE_NOT_DEFINED = 0x00,
> +	DP_CONTENT_TYPE_GRAPHICS = 0x01,
> +	DP_CONTENT_TYPE_PHOTO = 0x02,
> +	DP_CONTENT_TYPE_VIDEO = 0x03,
> +	DP_CONTENT_TYPE_GAME = 0x04,
> +};
> +
> +/* DRM DP VSC SDP as per DP 1.4 spec */ struct drm_dp_vsc_sdp {
> +	unsigned char sdp_type; /* Secondary-data Packet Type */
> +	unsigned char revision; /* Revision Number */

These comments seems self-explanatory, you can probably drop them.

> +	unsigned char length; /* Number of Valid Data Bytes */
> +	enum dp_colorspace colorspace;
> +	enum dp_colorimetry colorimetry;
> +	int bpc; /* bit per color */
> +	enum dp_dynamic_range dynamic_range;
> +	enum dp_content_type content_type;
> +};
> +
>  int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
> 
>  static inline int
> --
> 2.24.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Gwan-gyeong Mun Feb. 9, 2020, 3:26 a.m. UTC | #2 
On Wed, 2020-02-05 at 20:12 +0530, Shankar, Uma wrote:
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf
> > Of Gwan-
> > gyeong Mun
> > Sent: Tuesday, February 4, 2020 4:50 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: linux-fbdev@vger.kernel.org; dri-devel@lists.freedesktop.org
> > Subject: [PATCH v3 01/17] drm: add DP 1.4 VSC SDP Payload related
> > enums and a
> > structure
> 
> %s/add/Add/
> Also you can rephrase this as " Add DP1.4 VSC SDP Payload related
> Data Structures"/
> 
Hi Uma,
Thank you for reviewing a patch series.

Okay I'll rephrase commit message with your guide.

> > It adds new enumeration definitions for VSC SDP Payload for Pixel
> > Encoding/Colorimetry Format.
> > And it adds a new drm data structure for DP VSC SDP.
> > 
> > enum dp_colorspace and enum dp_colorimetry correspond "Pixel
> > Encoding and
> > Colorimetry Formats". enum dp_dynamic_range corresponds "Dynamic
> > Range".
> > And enum dp_content_type corresponds "Content Type"
> > All of them are based on DP 1.4 spec [Table 2-117: VSC SDP Payload
> > for
> > DB16 through DB18].
> > 
> > v3: Add a new drm data structure for DP VSC SDP
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> >  include/drm/drm_dp_helper.h | 57
> > +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> > 
> > diff --git a/include/drm/drm_dp_helper.h
> > b/include/drm/drm_dp_helper.h index
> > 262faf9e5e94..c098727681fa 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -1209,6 +1209,63 @@ struct dp_sdp {
> >  #define EDP_VSC_PSR_UPDATE_RFB		(1<<1)
> >  #define EDP_VSC_PSR_CRC_VALUES_VALID	(1<<2)
> > 
> > +/* Based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16
> > through
> > +DB18] */ enum dp_colorspace {
> 
> We should not call this colorspace, rather rename it to
> dp_pixelformat.
> These are not colorspaces.
> 
> Also add these to kernel docs as they are standard definitions.
> 
> > +	DP_COLORSPACE_RGB = 0,
> 
> Make this as DP_PIXELFORMAT_RGB and rename all the below ones as
> well.
> 
> > +	DP_COLORSPACE_YUV444 = 0x1,
> > +	DP_COLORSPACE_YUV422 = 0x2,
> > +	DP_COLORSPACE_YUV420 = 0x3,
> > +	DP_COLORSPACE_Y_ONLY = 0x4,
> > +	DP_COLORSPACE_RAW = 0x5,
> > +	DP_COLORSPACE_RESERVED = 0x6,
> > +};
> > +
> > +/**
> > + * Based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16
> > through
> > +DB18]
> > + * and a name of enum member followes DRM_MODE_COLORIMETRY
> > definition.
> 
> Typo in follows
> 
> > + */
> > +enum dp_colorimetry {
> 
> You can call this as dp_colorspace (this is actual colorspace), you
> can stick with colorimetry as well.
> Will leave that to you.
Yes, the DP spec uses colorimetry as the term, I prefer colorimetry.
> 
> > +	DP_COLORIMETRY_DEFAULT = 0, /* sRGB (IEC 61966-2-1) / ITU-R
> > BT.601 */
> > +	DP_COLORIMETRY_RGB_WIDE_FIXED = 0x1,
> > +	DP_COLORIMETRY_BT709_YCC = 0x1,
> > +	DP_COLORIMETRY_RGB_WIDE_FLOAT = 0x2,
> > +	DP_COLORIMETRY_XVYCC_601 = 0x2,
> > +	DP_COLORIMETRY_OPRGB = 0x3,
> > +	DP_COLORIMETRY_XVYCC_709 = 0x3,
> > +	DP_COLORIMETRY_DCI_P3_RGB = 0x4,
> > +	DP_COLORIMETRY_SYCC_601 = 0x4,
> > +	DP_COLORIMETRY_RGB_CUSTOM = 0x5,
> > +	DP_COLORIMETRY_OPYCC_601 = 0x5,
> > +	DP_COLORIMETRY_BT2020_RGB = 0x6,
> > +	DP_COLORIMETRY_BT2020_CYCC = 0x6,
> > +	DP_COLORIMETRY_BT2020_YCC = 0x7,
> > +};
> > +
> > +enum dp_dynamic_range {
> > +	DP_DYNAMIC_RANGE_VESA = 0,
> > +	DP_DYNAMIC_RANGE_CTA = 1,
> > +};
> > +
> > +enum dp_content_type {
> > +	DP_CONTENT_TYPE_NOT_DEFINED = 0x00,
> > +	DP_CONTENT_TYPE_GRAPHICS = 0x01,
> > +	DP_CONTENT_TYPE_PHOTO = 0x02,
> > +	DP_CONTENT_TYPE_VIDEO = 0x03,
> > +	DP_CONTENT_TYPE_GAME = 0x04,
> > +};
> > +
> > +/* DRM DP VSC SDP as per DP 1.4 spec */ struct drm_dp_vsc_sdp {
> > +	unsigned char sdp_type; /* Secondary-data Packet Type */
> > +	unsigned char revision; /* Revision Number */
> 
> These comments seems self-explanatory, you can probably drop them.
> 
Thank you for review in detail, I'll update everything that you commented.
> > +	unsigned char length; /* Number of Valid Data Bytes */
> > +	enum dp_colorspace colorspace;
> > +	enum dp_colorimetry colorimetry;
> > +	int bpc; /* bit per color */
> > +	enum dp_dynamic_range dynamic_range;
> > +	enum dp_content_type content_type;
> > +};
> > +
> >  int drm_dp_psr_setup_time(const u8
> > psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
> > 
> >  static inline int
> > --
> > 2.24.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 262faf9e5e94..c098727681fa 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1209,6 +1209,63 @@  struct dp_sdp {
 #define EDP_VSC_PSR_UPDATE_RFB		(1<<1)
 #define EDP_VSC_PSR_CRC_VALUES_VALID	(1<<2)
 
+/* Based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through DB18] */
+enum dp_colorspace {
+	DP_COLORSPACE_RGB = 0,
+	DP_COLORSPACE_YUV444 = 0x1,
+	DP_COLORSPACE_YUV422 = 0x2,
+	DP_COLORSPACE_YUV420 = 0x3,
+	DP_COLORSPACE_Y_ONLY = 0x4,
+	DP_COLORSPACE_RAW = 0x5,
+	DP_COLORSPACE_RESERVED = 0x6,
+};
+
+/**
+ * Based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through DB18]
+ * and a name of enum member followes DRM_MODE_COLORIMETRY definition.
+ */
+enum dp_colorimetry {
+	DP_COLORIMETRY_DEFAULT = 0, /* sRGB (IEC 61966-2-1) / ITU-R BT.601 */
+	DP_COLORIMETRY_RGB_WIDE_FIXED = 0x1,
+	DP_COLORIMETRY_BT709_YCC = 0x1,
+	DP_COLORIMETRY_RGB_WIDE_FLOAT = 0x2,
+	DP_COLORIMETRY_XVYCC_601 = 0x2,
+	DP_COLORIMETRY_OPRGB = 0x3,
+	DP_COLORIMETRY_XVYCC_709 = 0x3,
+	DP_COLORIMETRY_DCI_P3_RGB = 0x4,
+	DP_COLORIMETRY_SYCC_601 = 0x4,
+	DP_COLORIMETRY_RGB_CUSTOM = 0x5,
+	DP_COLORIMETRY_OPYCC_601 = 0x5,
+	DP_COLORIMETRY_BT2020_RGB = 0x6,
+	DP_COLORIMETRY_BT2020_CYCC = 0x6,
+	DP_COLORIMETRY_BT2020_YCC = 0x7,
+};
+
+enum dp_dynamic_range {
+	DP_DYNAMIC_RANGE_VESA = 0,
+	DP_DYNAMIC_RANGE_CTA = 1,
+};
+
+enum dp_content_type {
+	DP_CONTENT_TYPE_NOT_DEFINED = 0x00,
+	DP_CONTENT_TYPE_GRAPHICS = 0x01,
+	DP_CONTENT_TYPE_PHOTO = 0x02,
+	DP_CONTENT_TYPE_VIDEO = 0x03,
+	DP_CONTENT_TYPE_GAME = 0x04,
+};
+
+/* DRM DP VSC SDP as per DP 1.4 spec */
+struct drm_dp_vsc_sdp {
+	unsigned char sdp_type; /* Secondary-data Packet Type */
+	unsigned char revision; /* Revision Number */
+	unsigned char length; /* Number of Valid Data Bytes */
+	enum dp_colorspace colorspace;
+	enum dp_colorimetry colorimetry;
+	int bpc; /* bit per color */
+	enum dp_dynamic_range dynamic_range;
+	enum dp_content_type content_type;
+};
+
 int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
 
 static inline int