| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 4 Jan 2017 15:22:25 +0200
From: Ville Syrjälä <ville.syrjala@...ux.intel.com>
To: Jose Abreu <Jose.Abreu@...opsys.com>
Cc: dri-devel@...ts.freedesktop.org,
Carlos Palminha <CARLOS.PALMINHA@...opsys.com>,
linux-kernel@...r.kernel.org,
Daniel Vetter <daniel.vetter@...el.com>
Subject: Re: [RFC] drm: Parse HDMI 2.0 YCbCr 4:2:0 VDB and VCB
On Fri, Dec 30, 2016 at 04:53:16PM +0000, Jose Abreu wrote:
> HDMI 2.0 introduces a new sampling mode called YCbCr 4:2:0.
> According to the spec the EDID may contain two blocks that
> signal this sampling mode:
> - YCbCr 4:2:0 Video Data Block
> - YCbCr 4:2:0 Video Capability Map Data Block
>
> The video data block contains the list of vic's were
> only YCbCr 4:2:0 sampling mode shall be used while the
> video capability map data block contains a mask were
> YCbCr 4:2:0 sampling mode may be used.
>
> This RFC patch adds support for parsing these two new blocks
> and introduces new flags to signal the drivers if the
> mode is 4:2:0'only or 4:2:0'able.
>
> The reason this is still a RFC is because there is no
> reference in kernel for this new sampling mode (specially in
> AVI infoframe part), so, I was hoping to hear some feedback
> first.
>
> Tested in a HDMI 2.0 compliance scenario.
>
> Signed-off-by: Jose Abreu <joabreu@...opsys.com>
> Cc: Carlos Palminha <palminha@...opsys.com>
> Cc: Daniel Vetter <daniel.vetter@...el.com>
> Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> Cc: Sean Paul <seanpaul@...omium.org>
> Cc: David Airlie <airlied@...ux.ie>
> Cc: dri-devel@...ts.freedesktop.org
> Cc: linux-kernel@...r.kernel.org
> ---
> drivers/gpu/drm/drm_edid.c | 139 +++++++++++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/drm_modes.c | 10 +++-
> include/uapi/drm/drm_mode.h | 6 ++
> 3 files changed, 151 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 67d6a73..6ce1a38 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2549,6 +2549,8 @@ static int drm_cvt_modes(struct drm_connector *connector,
> #define VENDOR_BLOCK 0x03
> #define SPEAKER_BLOCK 0x04
> #define VIDEO_CAPABILITY_BLOCK 0x07
> +#define VIDEO_DATA_BLOCK_420 0x0E
> +#define VIDEO_CAP_BLOCK_420 0x0F
> #define EDID_BASIC_AUDIO (1 << 6)
> #define EDID_CEA_YCRCB444 (1 << 5)
> #define EDID_CEA_YCRCB422 (1 << 4)
> @@ -3050,6 +3052,98 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
> return modes;
> }
>
> +static int add_420_mode(struct drm_connector *connector, u8 vic)
> +{
> + struct drm_device *dev = connector->dev;
> + struct drm_display_mode *newmode;
> +
> + if (!drm_valid_cea_vic(vic))
> + return 0;
> +
> + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
> + if (!newmode)
> + return 0;
> +
> + newmode->flags |= DRM_MODE_FLAG_420_ONLY;
Why does userspace need to know this? My thinking has been that the
driver would do the right thing automagically.
We do probably want some kind of output colorspace property to allow the
user to select between RGB vs. YCbCr etc. But I think even with that we
should still allow the driver to automagically select YCbCr 4:2:0 output
since that's the only way the mode will work.
> + drm_mode_probed_add(connector, newmode);
> +
> + return 1;
> +}
> +
> +static int add_420_vdb_modes(struct drm_connector *connector, const u8 *svds,
> + u8 svds_len)
> +{
> + int modes = 0, i;
> +
> + for (i = 0; i < svds_len; i++)
> + modes += add_420_mode(connector, svds[i]);
> +
> + return modes;
> +}
> +
> +static int add_420_vcb_modes(struct drm_connector *connector, const u8 *svds,
> + u8 svds_len, const u8 *video_db, u8 video_len)
> +{
> + struct drm_display_mode *newmode = NULL;
> + int modes = 0, i, j;
> +
> + for (i = 0; i < svds_len; i++) {
> + u8 mask = svds[i];
> + for (j = 0; j < 8; j++) {
> + if (mask & (1 << j)) {
> + newmode = drm_display_mode_from_vic_index(
> + connector, video_db, video_len,
> + i * 8 + j);
> + if (newmode) {
> + newmode->flags |= DRM_MODE_FLAG_420;
> + drm_mode_probed_add(connector, newmode);
> + modes++;
> + }
> + }
> + }
> + }
> +
> + return modes;
> +}
> +
> +static int add_420_vcb_modes_all(struct drm_connector *connector,
> + const u8 *video_db, u8 video_len)
> +{
> + struct drm_display_mode *newmode = NULL;
> + int modes = 0, i;
> +
> + for (i = 0; i < video_len; i++) {
> + newmode = drm_display_mode_from_vic_index(connector, video_db,
> + video_len, i);
> + if (newmode) {
> + newmode->flags |= DRM_MODE_FLAG_420;
> + drm_mode_probed_add(connector, newmode);
> + modes++;
> + }
> + }
> +
> + return modes;
> +}
> +
> +static int do_hdmi_420_modes(struct drm_connector *connector, const u8 *vdb,
> + u8 vdb_len, const u8 *vcb, u8 vcb_len, const u8 *video_db,
> + u8 video_len)
> +{
> + int modes = 0;
> +
> + if (vdb && (vdb_len > 1)) /* Add 4:2:0 modes present in EDID */
> + modes += add_420_vdb_modes(connector, &vdb[2], vdb_len - 1);
> +
> + if (vcb && (vcb_len > 1)) /* Parse bit mask of supported modes */
> + modes += add_420_vcb_modes(connector, &vcb[2], vcb_len - 1,
> + video_db, video_len);
> + else if (vcb) /* All modes support 4:2:0 mode */
> + modes += add_420_vcb_modes_all(connector, video_db, video_len);
> +
> + DRM_DEBUG("added %d 4:2:0 modes\n", modes);
> + return modes;
> +}
> +
> /*
> * do_hdmi_vsdb_modes - Parse the HDMI Vendor Specific data block
> * @connector: connector corresponding to the HDMI sink
> @@ -3206,6 +3300,12 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure,
> }
>
> static int
> +cea_db_extended_tag(const u8 *db)
> +{
> + return db[1];
> +}
> +
> +static int
> cea_revision(const u8 *cea)
> {
> return cea[1];
> @@ -3239,6 +3339,28 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
> return hdmi_id == HDMI_IEEE_OUI;
> }
>
> +static bool cea_db_is_hdmi_vdb420(const u8 *db)
> +{
> + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK)
> + return false;
> +
> + if (cea_db_extended_tag(db) != VIDEO_DATA_BLOCK_420)
> + return false;
> +
> + return true;
> +}
> +
> +static bool cea_db_is_hdmi_vcb420(const u8 *db)
> +{
> + if (cea_db_tag(db) != VIDEO_CAPABILITY_BLOCK)
> + return false;
> +
> + if (cea_db_extended_tag(db) != VIDEO_CAP_BLOCK_420)
> + return false;
> +
> + return true;
> +}
> +
> #define for_each_cea_db(cea, i, start, end) \
> for ((i) = (start); (i) < (end) && (i) + cea_db_payload_len(&(cea)[(i)]) < (end); (i) += cea_db_payload_len(&(cea)[(i)]) + 1)
>
> @@ -3246,8 +3368,9 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
> add_cea_modes(struct drm_connector *connector, struct edid *edid)
> {
> const u8 *cea = drm_find_cea_extension(edid);
> - const u8 *db, *hdmi = NULL, *video = NULL;
> - u8 dbl, hdmi_len, video_len = 0;
> + const u8 *db, *hdmi = NULL, *video = NULL, *vdb420 = NULL,
> + *vcb420 = NULL;
> + u8 dbl, hdmi_len, video_len = 0, vdb420_len = 0, vcb420_len = 0;
> int modes = 0;
>
> if (cea && cea_revision(cea) >= 3) {
> @@ -3269,6 +3392,14 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
> hdmi = db;
> hdmi_len = dbl;
> }
> + else if (cea_db_is_hdmi_vdb420(db)) {
> + vdb420 = db;
> + vdb420_len = dbl;
> + }
> + else if (cea_db_is_hdmi_vcb420(db)) {
> + vcb420 = db;
> + vcb420_len = dbl;
> + }
> }
> }
>
> @@ -3280,6 +3411,10 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
> modes += do_hdmi_vsdb_modes(connector, hdmi, hdmi_len, video,
> video_len);
>
> + if (vdb420 || vcb420)
> + modes += do_hdmi_420_modes(connector, vdb420, vdb420_len,
> + vcb420, vcb420_len, video, video_len);
> +
> return modes;
> }
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index ac6a352..53c65f6 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -967,6 +967,10 @@ bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct
> (mode2->flags & DRM_MODE_FLAG_3D_MASK))
> return false;
>
> + if ((mode1->flags & DRM_MODE_FLAG_420_MASK) !=
> + (mode2->flags & DRM_MODE_FLAG_420_MASK))
> + return false;
> +
> return drm_mode_equal_no_clocks_no_stereo(mode1, mode2);
> }
> EXPORT_SYMBOL(drm_mode_equal_no_clocks);
> @@ -985,6 +989,9 @@ bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct
> bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
> const struct drm_display_mode *mode2)
> {
> + unsigned int flags_mask =
> + ~(DRM_MODE_FLAG_3D_MASK | DRM_MODE_FLAG_420_MASK);
> +
> if (mode1->hdisplay == mode2->hdisplay &&
> mode1->hsync_start == mode2->hsync_start &&
> mode1->hsync_end == mode2->hsync_end &&
> @@ -995,8 +1002,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
> mode1->vsync_end == mode2->vsync_end &&
> mode1->vtotal == mode2->vtotal &&
> mode1->vscan == mode2->vscan &&
> - (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) ==
> - (mode2->flags & ~DRM_MODE_FLAG_3D_MASK))
> + (mode1->flags & flags_mask) == (mode2->flags & flags_mask))
> return true;
>
> return false;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index ce7efe2..dc8e285 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -84,6 +84,12 @@
> #define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH (6<<14)
> #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14)
> #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14)
> +/*
> + * HDMI 2.0
> + */
> +#define DRM_MODE_FLAG_420_MASK (0x03<<19)
> +#define DRM_MODE_FLAG_420 (1<<19)
> +#define DRM_MODE_FLAG_420_ONLY (1<<20)
>
> /* Picture aspect ratio options */
> #define DRM_MODE_PICTURE_ASPECT_NONE 0
> --
> 1.9.1
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
Powered by blists - more mailing lists