* [PATCH v4 1/2] drm/edid: Complete CEA modedb(VIC 1-107)
@ 2017-03-23 15:14 Shashank Sharma
2017-03-23 15:14 ` [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames Shashank Sharma
2017-03-23 16:01 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/2] drm/edid: Complete CEA modedb(VIC 1-107) Patchwork
0 siblings, 2 replies; 22+ messages in thread
From: Shashank Sharma @ 2017-03-23 15:14 UTC (permalink / raw)
To: dri-devel, intel-gfx; +Cc: Jose Abreu, Alex Deucher
CEA-861-F specs defines new video modes to be used with
HDMI 2.0 EDIDs. The VIC range has been extended from 1-64 to
1-107.
Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now
to be able to parse new CEA modes using the existing methods, we have
to complete the modedb (VIC=65 onwards).
This patch adds:
- Timings for existing CEA video modes (from VIC=65 till VIC=92)
- Newly added 4k modes (from VIC=93 to VIC=107).
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
V2: Addressed review comments from Jose:
- fix the timings for VIC 83, 90 and 91
- fix formatting for VIC 93-107
V3: Rebase on drm-tip, added R-B from Jose, Alex.
V4: Addressed review comments from Andrzej not to modify the
VIC filed for HDMI 1.4b sinks (by adding another patch).
Reviewed-by: Jose Abreu <Jose.Abreu@synopsys.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/drm_edid.c | 215 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 215 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e66397a..3b494c2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1001,6 +1001,221 @@ static const struct drm_display_mode edid_cea_modes[] = {
2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
.vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
+ /* 65 - 1280x720@24Hz */
+ { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 59400, 1280, 3040,
+ 3080, 3300, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 66 - 1280x720@25Hz */
+ { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3700,
+ 3740, 3960, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 67 - 1280x720@30Hz */
+ { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 3040,
+ 3080, 3300, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 68 - 1280x720@50Hz */
+ { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1720,
+ 1760, 1980, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 69 - 1280x720@60Hz */
+ { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 74250, 1280, 1390,
+ 1430, 1650, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 70 - 1280x720@100Hz */
+ { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1720,
+ 1760, 1980, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 71 - 1280x720@120Hz */
+ { DRM_MODE("1280x720", DRM_MODE_TYPE_DRIVER, 148500, 1280, 1390,
+ 1430, 1650, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 72 - 1920x1080@24Hz */
+ { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2558,
+ 2602, 2750, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 73 - 1920x1080@25Hz */
+ { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2448,
+ 2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 74 - 1920x1080@30Hz */
+ { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 74250, 1920, 2008,
+ 2052, 2200, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 75 - 1920x1080@50Hz */
+ { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2448,
+ 2492, 2640, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 76 - 1920x1080@60Hz */
+ { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 148500, 1920, 2008,
+ 2052, 2200, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 77 - 1920x1080@100Hz */
+ { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 297000, 1920, 2448,
+ 2492, 2640, 0, 1080, 1084, 1094, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 78 - 1920x1080@120Hz */
+ { DRM_MODE("1920x1080", DRM_MODE_TYPE_DRIVER, 297000, 1920, 2008,
+ 2052, 2200, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 79 - 1680x720@24Hz */
+ { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 3040,
+ 3080, 3300, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 80 - 1680x720@25Hz */
+ { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 2908,
+ 2948, 3168, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 81 - 1680x720@30Hz */
+ { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 59400, 1680, 2380,
+ 2420, 2640, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 82 - 1680x720@50Hz */
+ { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 82500, 1680, 1940,
+ 1980, 2200, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 83 - 1680x720@60Hz */
+ { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 99000, 1680, 1940,
+ 1980, 2200, 0, 720, 725, 730, 750, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 84 - 1680x720@100Hz */
+ { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 165000, 1680, 1740,
+ 1780, 2000, 0, 720, 725, 730, 825, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 85 - 1680x720@120Hz */
+ { DRM_MODE("1680x720", DRM_MODE_TYPE_DRIVER, 198000, 1680, 1740,
+ 1780, 2000, 0, 720, 725, 730, 825, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 86 - 2560x1080@24Hz */
+ { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 99000, 2560, 3558,
+ 3602, 3750, 0, 1080, 1084, 1089, 1100, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 87 - 2560x1080@25Hz */
+ { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 90000, 2560, 3008,
+ 3052, 3200, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 88 - 2560x1080@30Hz */
+ { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 118800, 2560, 3328,
+ 3372, 3520, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 89 - 2560x1080@50Hz */
+ { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 185625, 2560, 3108,
+ 3152, 3300, 0, 1080, 1084, 1089, 1125, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 90 - 2560x1080@60Hz */
+ { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 198000, 2560, 2808,
+ 2852, 3000, 0, 1080, 1084, 1089, 1100, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 91 - 2560x1080@100Hz */
+ { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 371250, 2560, 2778,
+ 2822, 2970, 0, 1080, 1084, 1089, 1250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 100, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 92 - 2560x1080@120Hz */
+ { DRM_MODE("2560x1080", DRM_MODE_TYPE_DRIVER, 495000, 2560, 3108,
+ 3152, 3300, 0, 1080, 1084, 1089, 1250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 120, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27, },
+ /* 93 - 3840x2160p@24Hz 16:9 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 5116,
+ 5204, 5500, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9,},
+ /* 94 - 3840x2160p@25Hz 16:9 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4896,
+ 4984, 5280, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9},
+ /* 95 - 3840x2160p@30Hz 16:9 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016,
+ 4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9},
+ /* 96 - 3840x2160p@50Hz 16:9 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4896,
+ 4984, 5280, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9},
+ /* 97 - 3840x2160p@60Hz 16:9 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4016,
+ 4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9},
+ /* 98 - 4096x2160p@24Hz 256:135 */
+ { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 5116,
+ 5204, 5500, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+ /* 99 - 4096x2160p@25Hz 256:135 */
+ { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 5064,
+ 5152, 5280, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+ /* 100 - 4096x2160p@30Hz 256:135 */
+ { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 297000, 4096, 4184,
+ 4272, 4400, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+ /* 101 - 4096x2160p@50Hz 256:135 */
+ { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 594000, 4096, 5064,
+ 5152, 5280, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+ /* 102 - 4096x2160p@60Hz 256:135 */
+ { DRM_MODE("4096x2160", DRM_MODE_TYPE_DRIVER, 594000, 4096, 4184,
+ 4272, 4400, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135},
+ /* 103 - 3840x2160p@24Hz 64:27 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 5116,
+ 5204, 5500, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
+ /* 104 - 3840x2160p@25Hz 64:27 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016,
+ 4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
+ /* 105 - 3840x2160p@30Hz 64:27 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016,
+ 4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
+ /* 106 - 3840x2160p@50Hz 64:27 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4896,
+ 4984, 5280, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
+ /* 107 - 3840x2160p@60Hz 64:27 */
+ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 594000, 3840, 4016,
+ 4104, 4400, 0, 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27},
};
/*
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:14 [PATCH v4 1/2] drm/edid: Complete CEA modedb(VIC 1-107) Shashank Sharma
@ 2017-03-23 15:14 ` Shashank Sharma
2017-03-23 15:17 ` Ilia Mirkin
` (2 more replies)
2017-03-23 16:01 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/2] drm/edid: Complete CEA modedb(VIC 1-107) Patchwork
1 sibling, 3 replies; 22+ messages in thread
From: Shashank Sharma @ 2017-03-23 15:14 UTC (permalink / raw)
To: dri-devel, intel-gfx
Cc: Jose Abreu, Andrzej Hajda, Alex Deucher, Daniel Vetter
HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
For any other mode, the VIC filed in AVI infoframes should be 0.
HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
extended to (VIC 1-107).
This patch adds a bool input variable, which indicates if the connected
sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
HDMI 2.0 VIC to a HDMI 1.4 sink.
This patch touches all drm drivers, who are callers of this function
drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
no change in current behavior, is_hdmi2 is kept as false.
In case of I915 driver, this patch checks the connector->display_info
to check if the connected display is HDMI 2.0.
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Jose Abreu <jose.abreu@synopsys.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
PS: This patch touches a few lines in few files, which were
already above 80 char, so checkpatch gives 80 char warning again.
- gpu/drm/omapdrm/omap_encoder.c
- gpu/drm/i915/intel_sdvo.c
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
drivers/gpu/drm/bridge/sii902x.c | 2 +-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
drivers/gpu/drm/tegra/hdmi.c | 2 +-
drivers/gpu/drm/tegra/sor.c | 2 +-
drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
include/drm/drm_edid.h | 3 ++-
21 files changed, 38 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index d4452d8..ab7a399 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
dce_v10_0_audio_write_sad_regs(encoder);
dce_v10_0_audio_write_latency_fields(encoder, mode);
- err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+ err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 5b24e89..3c5fd83 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
dce_v11_0_audio_write_sad_regs(encoder);
dce_v11_0_audio_write_latency_fields(encoder, mode);
- err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+ err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index d2590d7..c564dca 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
dce_v8_0_audio_write_sad_regs(encoder);
dce_v8_0_audio_write_latency_fields(encoder, mode);
- err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+ err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index a2a8236..f9b77b8 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
mutex_lock(&anx78xx->lock);
- err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode);
+ err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
+ false);
if (err) {
DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
goto unlock;
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 9126d03..dcf15d7 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -269,7 +269,7 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
if (ret)
return;
- ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
+ ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
if (ret < 0) {
DRM_ERROR("couldn't fill AVI infoframe\n");
return;
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index af93f7a..5ff2886 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
u8 val;
/* Initialise info frame from DRM mode */
- drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+ drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (hdmi->hdmi_data.enc_out_format == YCBCR444)
frame.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3b494c2..f9a4b08 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4664,12 +4664,14 @@ EXPORT_SYMBOL(drm_set_preferred_mode);
* data from a DRM display mode
* @frame: HDMI AVI infoframe
* @mode: DRM display mode
+ * @is_hdmi2: Sink is HDMI 2.0 compliant
*
* Return: 0 on success or a negative error code on failure.
*/
int
drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
- const struct drm_display_mode *mode)
+ const struct drm_display_mode *mode,
+ bool is_hdmi2)
{
int err;
@@ -4685,6 +4687,14 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
frame->video_code = drm_match_cea_mode(mode);
+ /*
+ * HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
+ * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
+ * have to make sure we dont break HDMI 1.4 sinks.
+ */
+ if (!is_hdmi2 && frame->video_code > 64)
+ frame->video_code = 0;
+
frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
/*
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5243840..5cb44d4 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -781,7 +781,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
}
ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
- &hdata->current_mode);
+ &hdata->current_mode, false);
if (!ret)
ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
if (ret > 0) {
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 86f47e1..d1e7ac5 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -712,7 +712,7 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
{
union hdmi_infoframe frame;
- drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+ drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 3eec74c..96644f3 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -458,11 +458,14 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
const struct drm_display_mode *adjusted_mode =
&crtc_state->base.adjusted_mode;
+ struct drm_connector *connector = &intel_hdmi->attached_connector->base;
+ bool is_hdmi2 = connector->display_info.hdmi.scdc.supported;
union hdmi_infoframe frame;
int ret;
ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
- adjusted_mode);
+ adjusted_mode,
+ is_hdmi2);
if (ret < 0) {
DRM_ERROR("couldn't fill AVI infoframe\n");
return;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 816a6f5..694f626 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1011,7 +1011,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
ssize_t len;
ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
- &pipe_config->base.adjusted_mode);
+ &pipe_config->base.adjusted_mode,
+ false);
if (ret < 0) {
DRM_ERROR("couldn't fill AVI infoframe\n");
return false;
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index c262512..80c36af 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -975,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
u8 buffer[17];
ssize_t err;
- err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+ err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
dev_err(hdmi->dev,
"Failed to get AVI infoframe from mode: %zd\n", err);
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 86c977b..624f5b5 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -85,7 +85,8 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
if (hdmi_mode && dssdev->driver->set_hdmi_infoframe) {
struct hdmi_avi_infoframe avi;
- r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode);
+ r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
+ false);
if (r == 0)
dssdev->driver->set_hdmi_infoframe(dssdev, &avi);
}
diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index b214663..49b42bf 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
if (!connector)
return -EINVAL;
- err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+ err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
DRM_ERROR("failed to setup AVI infoframe: %d\n", err);
return err;
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 006260d..b105f1c 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -294,7 +294,7 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
union hdmi_infoframe frame;
int rc;
- rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+ rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index ce2dcba..2128b8c 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -434,7 +434,7 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
DRM_DEBUG_DRIVER("\n");
- ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode);
+ ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false);
if (ret < 0) {
DRM_ERROR("failed to setup AVI infoframe: %d\n", ret);
return ret;
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index cda0491..718d8db 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -734,7 +734,7 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
u8 buffer[17];
ssize_t err;
- err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+ err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err);
return;
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index a8f5289..fb2709c 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1904,7 +1904,7 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
value &= ~INFOFRAME_CTRL_ENABLE;
tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL);
- err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
+ err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
if (err < 0) {
dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
return err;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index e9cbe26..5d81301 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -394,7 +394,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
union hdmi_infoframe frame;
int ret;
- ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+ ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
if (ret < 0) {
DRM_ERROR("couldn't fill AVI infoframe\n");
return;
diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
index c47b9cb..3e267dd 100644
--- a/drivers/gpu/drm/zte/zx_hdmi.c
+++ b/drivers/gpu/drm/zte/zx_hdmi.c
@@ -125,7 +125,7 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
union hdmi_infoframe frame;
int ret;
- ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
+ ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
if (ret) {
DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n",
ret);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index a21d494..a4d174e7 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -348,7 +348,8 @@ drm_load_edid_firmware(struct drm_connector *connector)
int
drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
- const struct drm_display_mode *mode);
+ const struct drm_display_mode *mode,
+ bool is_hdmi2);
int
drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
const struct drm_display_mode *mode);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:14 ` [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames Shashank Sharma
@ 2017-03-23 15:17 ` Ilia Mirkin
2017-03-23 15:22 ` Sharma, Shashank
2017-03-23 15:28 ` Jose Abreu
2017-03-23 15:36 ` Ville Syrjälä
2 siblings, 1 reply; 22+ messages in thread
From: Ilia Mirkin @ 2017-03-23 15:17 UTC (permalink / raw)
To: Shashank Sharma
Cc: Jose Abreu, Alex Deucher, Intel Graphics Development, dri-devel,
Daniel Vetter
On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma
<shashank.sharma@intel.com> wrote:
> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
> For any other mode, the VIC filed in AVI infoframes should be 0.
> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
> extended to (VIC 1-107).
>
> This patch adds a bool input variable, which indicates if the connected
> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
> HDMI 2.0 VIC to a HDMI 1.4 sink.
Should this patch come before the patch which recognizes VICs 65+?
Otherwise if only the first patch is applied but not this one, you
could end up with the bad scenario. (As can happen in bisections, for
example.)
>
> This patch touches all drm drivers, who are callers of this function
> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
> no change in current behavior, is_hdmi2 is kept as false.
>
> In case of I915 driver, this patch checks the connector->display_info
> to check if the connected display is HDMI 2.0.
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jose Abreu <jose.abreu@synopsys.com>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
>
> PS: This patch touches a few lines in few files, which were
> already above 80 char, so checkpatch gives 80 char warning again.
> - gpu/drm/omapdrm/omap_encoder.c
> - gpu/drm/i915/intel_sdvo.c
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
> drivers/gpu/drm/tegra/hdmi.c | 2 +-
> drivers/gpu/drm/tegra/sor.c | 2 +-
> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> include/drm/drm_edid.h | 3 ++-
> 21 files changed, 38 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index d4452d8..ab7a399 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
> dce_v10_0_audio_write_sad_regs(encoder);
> dce_v10_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 5b24e89..3c5fd83 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
> dce_v11_0_audio_write_sad_regs(encoder);
> dce_v11_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index d2590d7..c564dca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
> dce_v8_0_audio_write_sad_regs(encoder);
> dce_v8_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> index a2a8236..f9b77b8 100644
> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> @@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
>
> mutex_lock(&anx78xx->lock);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
> + false);
> if (err) {
> DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
> goto unlock;
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 9126d03..dcf15d7 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -269,7 +269,7 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> if (ret)
> return;
>
> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return;
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index af93f7a..5ff2886 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
> u8 val;
>
> /* Initialise info frame from DRM mode */
> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>
> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> frame.colorspace = HDMI_COLORSPACE_YUV444;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3b494c2..f9a4b08 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4664,12 +4664,14 @@ EXPORT_SYMBOL(drm_set_preferred_mode);
> * data from a DRM display mode
> * @frame: HDMI AVI infoframe
> * @mode: DRM display mode
> + * @is_hdmi2: Sink is HDMI 2.0 compliant
> *
> * Return: 0 on success or a negative error code on failure.
> */
> int
> drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> - const struct drm_display_mode *mode)
> + const struct drm_display_mode *mode,
> + bool is_hdmi2)
> {
> int err;
>
> @@ -4685,6 +4687,14 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>
> frame->video_code = drm_match_cea_mode(mode);
>
> + /*
> + * HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
> + * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
> + * have to make sure we dont break HDMI 1.4 sinks.
> + */
> + if (!is_hdmi2 && frame->video_code > 64)
> + frame->video_code = 0;
> +
> frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
>
> /*
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 5243840..5cb44d4 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -781,7 +781,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
> }
>
> ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> - &hdata->current_mode);
> + &hdata->current_mode, false);
> if (!ret)
> ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
> if (ret > 0) {
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 86f47e1..d1e7ac5 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -712,7 +712,7 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
> {
> union hdmi_infoframe frame;
>
> - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
>
> tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 3eec74c..96644f3 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -458,11 +458,14 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> const struct drm_display_mode *adjusted_mode =
> &crtc_state->base.adjusted_mode;
> + struct drm_connector *connector = &intel_hdmi->attached_connector->base;
> + bool is_hdmi2 = connector->display_info.hdmi.scdc.supported;
> union hdmi_infoframe frame;
> int ret;
>
> ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> - adjusted_mode);
> + adjusted_mode,
> + is_hdmi2);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return;
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 816a6f5..694f626 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1011,7 +1011,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> ssize_t len;
>
> ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> - &pipe_config->base.adjusted_mode);
> + &pipe_config->base.adjusted_mode,
> + false);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return false;
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index c262512..80c36af 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -975,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
> u8 buffer[17];
> ssize_t err;
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> dev_err(hdmi->dev,
> "Failed to get AVI infoframe from mode: %zd\n", err);
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 86c977b..624f5b5 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -85,7 +85,8 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
> if (hdmi_mode && dssdev->driver->set_hdmi_infoframe) {
> struct hdmi_avi_infoframe avi;
>
> - r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode);
> + r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
> + false);
> if (r == 0)
> dssdev->driver->set_hdmi_infoframe(dssdev, &avi);
> }
> diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
> index b214663..49b42bf 100644
> --- a/drivers/gpu/drm/radeon/radeon_audio.c
> +++ b/drivers/gpu/drm/radeon/radeon_audio.c
> @@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
> if (!connector)
> return -EINVAL;
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %d\n", err);
> return err;
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 006260d..b105f1c 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -294,7 +294,7 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
> union hdmi_infoframe frame;
> int rc;
>
> - rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> + rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
>
> if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
> frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index ce2dcba..2128b8c 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -434,7 +434,7 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
>
> DRM_DEBUG_DRIVER("\n");
>
> - ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode);
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false);
> if (ret < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %d\n", ret);
> return ret;
> diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
> index cda0491..718d8db 100644
> --- a/drivers/gpu/drm/tegra/hdmi.c
> +++ b/drivers/gpu/drm/tegra/hdmi.c
> @@ -734,7 +734,7 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
> u8 buffer[17];
> ssize_t err;
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index a8f5289..fb2709c 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -1904,7 +1904,7 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
> value &= ~INFOFRAME_CTRL_ENABLE;
> tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
> return err;
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index e9cbe26..5d81301 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -394,7 +394,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
> union hdmi_infoframe frame;
> int ret;
>
> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return;
> diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
> index c47b9cb..3e267dd 100644
> --- a/drivers/gpu/drm/zte/zx_hdmi.c
> +++ b/drivers/gpu/drm/zte/zx_hdmi.c
> @@ -125,7 +125,7 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
> union hdmi_infoframe frame;
> int ret;
>
> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> if (ret) {
> DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n",
> ret);
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index a21d494..a4d174e7 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -348,7 +348,8 @@ drm_load_edid_firmware(struct drm_connector *connector)
>
> int
> drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> - const struct drm_display_mode *mode);
> + const struct drm_display_mode *mode,
> + bool is_hdmi2);
> int
> drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
> const struct drm_display_mode *mode);
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:17 ` Ilia Mirkin
@ 2017-03-23 15:22 ` Sharma, Shashank
2017-03-23 15:28 ` Ilia Mirkin
0 siblings, 1 reply; 22+ messages in thread
From: Sharma, Shashank @ 2017-03-23 15:22 UTC (permalink / raw)
To: Ilia Mirkin
Cc: Jose Abreu, Alex Deucher, Intel Graphics Development, dri-devel,
Daniel Vetter
Regards
Shashank
On 3/23/2017 5:17 PM, Ilia Mirkin wrote:
> On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma
> <shashank.sharma@intel.com> wrote:
>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
>> For any other mode, the VIC filed in AVI infoframes should be 0.
>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>> extended to (VIC 1-107).
>>
>> This patch adds a bool input variable, which indicates if the connected
>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>> HDMI 2.0 VIC to a HDMI 1.4 sink.
> Should this patch come before the patch which recognizes VICs 65+?
> Otherwise if only the first patch is applied but not this one, you
> could end up with the bad scenario. (As can happen in bisections, for
> example.)
I kindof agree, this could be the case, but also, for the correct
functionality, you should have the whole series
together.
>> This patch touches all drm drivers, who are callers of this function
>> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
>> no change in current behavior, is_hdmi2 is kept as false.
>>
>> In case of I915 driver, this patch checks the connector->display_info
>> to check if the connected display is HDMI 2.0.
>>
>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>
>> PS: This patch touches a few lines in few files, which were
>> already above 80 char, so checkpatch gives 80 char warning again.
>> - gpu/drm/omapdrm/omap_encoder.c
>> - gpu/drm/i915/intel_sdvo.c
>>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>> drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
>> drivers/gpu/drm/tegra/sor.c | 2 +-
>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
>> include/drm/drm_edid.h | 3 ++-
>> 21 files changed, 38 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> index d4452d8..ab7a399 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
>> dce_v10_0_audio_write_sad_regs(encoder);
>> dce_v10_0_audio_write_latency_fields(encoder, mode);
>>
>> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>> if (err < 0) {
>> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>> return;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> index 5b24e89..3c5fd83 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
>> @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
>> dce_v11_0_audio_write_sad_regs(encoder);
>> dce_v11_0_audio_write_latency_fields(encoder, mode);
>>
>> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>> if (err < 0) {
>> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>> return;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> index d2590d7..c564dca 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
>> @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>> dce_v8_0_audio_write_sad_regs(encoder);
>> dce_v8_0_audio_write_latency_fields(encoder, mode);
>>
>> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>> if (err < 0) {
>> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>> return;
>> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
>> index a2a8236..f9b77b8 100644
>> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
>> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
>> @@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
>>
>> mutex_lock(&anx78xx->lock);
>>
>> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode);
>> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
>> + false);
>> if (err) {
>> DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
>> goto unlock;
>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
>> index 9126d03..dcf15d7 100644
>> --- a/drivers/gpu/drm/bridge/sii902x.c
>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>> @@ -269,7 +269,7 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>> if (ret)
>> return;
>>
>> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
>> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
>> if (ret < 0) {
>> DRM_ERROR("couldn't fill AVI infoframe\n");
>> return;
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index af93f7a..5ff2886 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>> u8 val;
>>
>> /* Initialise info frame from DRM mode */
>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>>
>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 3b494c2..f9a4b08 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4664,12 +4664,14 @@ EXPORT_SYMBOL(drm_set_preferred_mode);
>> * data from a DRM display mode
>> * @frame: HDMI AVI infoframe
>> * @mode: DRM display mode
>> + * @is_hdmi2: Sink is HDMI 2.0 compliant
>> *
>> * Return: 0 on success or a negative error code on failure.
>> */
>> int
>> drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>> - const struct drm_display_mode *mode)
>> + const struct drm_display_mode *mode,
>> + bool is_hdmi2)
>> {
>> int err;
>>
>> @@ -4685,6 +4687,14 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>>
>> frame->video_code = drm_match_cea_mode(mode);
>>
>> + /*
>> + * HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
>> + * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
>> + * have to make sure we dont break HDMI 1.4 sinks.
>> + */
>> + if (!is_hdmi2 && frame->video_code > 64)
>> + frame->video_code = 0;
>> +
>> frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
>>
>> /*
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 5243840..5cb44d4 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -781,7 +781,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>> }
>>
>> ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>> - &hdata->current_mode);
>> + &hdata->current_mode, false);
>> if (!ret)
>> ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
>> if (ret > 0) {
>> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
>> index 86f47e1..d1e7ac5 100644
>> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
>> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
>> @@ -712,7 +712,7 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
>> {
>> union hdmi_infoframe frame;
>>
>> - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
>> + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
>> frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
>>
>> tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 3eec74c..96644f3 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -458,11 +458,14 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>> const struct drm_display_mode *adjusted_mode =
>> &crtc_state->base.adjusted_mode;
>> + struct drm_connector *connector = &intel_hdmi->attached_connector->base;
>> + bool is_hdmi2 = connector->display_info.hdmi.scdc.supported;
>> union hdmi_infoframe frame;
>> int ret;
>>
>> ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
>> - adjusted_mode);
>> + adjusted_mode,
>> + is_hdmi2);
>> if (ret < 0) {
>> DRM_ERROR("couldn't fill AVI infoframe\n");
>> return;
>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>> index 816a6f5..694f626 100644
>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>> @@ -1011,7 +1011,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
>> ssize_t len;
>>
>> ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
>> - &pipe_config->base.adjusted_mode);
>> + &pipe_config->base.adjusted_mode,
>> + false);
>> if (ret < 0) {
>> DRM_ERROR("couldn't fill AVI infoframe\n");
>> return false;
>> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> index c262512..80c36af 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> @@ -975,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
>> u8 buffer[17];
>> ssize_t err;
>>
>> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>> if (err < 0) {
>> dev_err(hdmi->dev,
>> "Failed to get AVI infoframe from mode: %zd\n", err);
>> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
>> index 86c977b..624f5b5 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
>> @@ -85,7 +85,8 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
>> if (hdmi_mode && dssdev->driver->set_hdmi_infoframe) {
>> struct hdmi_avi_infoframe avi;
>>
>> - r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode);
>> + r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
>> + false);
>> if (r == 0)
>> dssdev->driver->set_hdmi_infoframe(dssdev, &avi);
>> }
>> diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
>> index b214663..49b42bf 100644
>> --- a/drivers/gpu/drm/radeon/radeon_audio.c
>> +++ b/drivers/gpu/drm/radeon/radeon_audio.c
>> @@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
>> if (!connector)
>> return -EINVAL;
>>
>> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>> if (err < 0) {
>> DRM_ERROR("failed to setup AVI infoframe: %d\n", err);
>> return err;
>> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
>> index 006260d..b105f1c 100644
>> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
>> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
>> @@ -294,7 +294,7 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
>> union hdmi_infoframe frame;
>> int rc;
>>
>> - rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
>> + rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
>>
>> if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
>> frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
>> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
>> index ce2dcba..2128b8c 100644
>> --- a/drivers/gpu/drm/sti/sti_hdmi.c
>> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
>> @@ -434,7 +434,7 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
>>
>> DRM_DEBUG_DRIVER("\n");
>>
>> - ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode);
>> + ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false);
>> if (ret < 0) {
>> DRM_ERROR("failed to setup AVI infoframe: %d\n", ret);
>> return ret;
>> diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
>> index cda0491..718d8db 100644
>> --- a/drivers/gpu/drm/tegra/hdmi.c
>> +++ b/drivers/gpu/drm/tegra/hdmi.c
>> @@ -734,7 +734,7 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
>> u8 buffer[17];
>> ssize_t err;
>>
>> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>> if (err < 0) {
>> dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err);
>> return;
>> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
>> index a8f5289..fb2709c 100644
>> --- a/drivers/gpu/drm/tegra/sor.c
>> +++ b/drivers/gpu/drm/tegra/sor.c
>> @@ -1904,7 +1904,7 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
>> value &= ~INFOFRAME_CTRL_ENABLE;
>> tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL);
>>
>> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>> if (err < 0) {
>> dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
>> return err;
>> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
>> index e9cbe26..5d81301 100644
>> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
>> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
>> @@ -394,7 +394,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
>> union hdmi_infoframe frame;
>> int ret;
>>
>> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
>> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
>> if (ret < 0) {
>> DRM_ERROR("couldn't fill AVI infoframe\n");
>> return;
>> diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
>> index c47b9cb..3e267dd 100644
>> --- a/drivers/gpu/drm/zte/zx_hdmi.c
>> +++ b/drivers/gpu/drm/zte/zx_hdmi.c
>> @@ -125,7 +125,7 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
>> union hdmi_infoframe frame;
>> int ret;
>>
>> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
>> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
>> if (ret) {
>> DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n",
>> ret);
>> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
>> index a21d494..a4d174e7 100644
>> --- a/include/drm/drm_edid.h
>> +++ b/include/drm/drm_edid.h
>> @@ -348,7 +348,8 @@ drm_load_edid_firmware(struct drm_connector *connector)
>>
>> int
>> drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>> - const struct drm_display_mode *mode);
>> + const struct drm_display_mode *mode,
>> + bool is_hdmi2);
>> int
>> drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>> const struct drm_display_mode *mode);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:14 ` [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames Shashank Sharma
2017-03-23 15:17 ` Ilia Mirkin
@ 2017-03-23 15:28 ` Jose Abreu
2017-03-23 15:45 ` Ville Syrjälä
2017-03-23 15:36 ` Ville Syrjälä
2 siblings, 1 reply; 22+ messages in thread
From: Jose Abreu @ 2017-03-23 15:28 UTC (permalink / raw)
To: Shashank Sharma, dri-devel, intel-gfx
Cc: Jose Abreu, Andrzej Hajda, Alex Deucher, Daniel Vetter
Hi Shashank,
On 23-03-2017 15:14, Shashank Sharma wrote:
> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
> For any other mode, the VIC filed in AVI infoframes should be 0.
> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
> extended to (VIC 1-107).
>
> This patch adds a bool input variable, which indicates if the connected
> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
> HDMI 2.0 VIC to a HDMI 1.4 sink.
>
> This patch touches all drm drivers, who are callers of this function
> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
> no change in current behavior, is_hdmi2 is kept as false.
>
> In case of I915 driver, this patch checks the connector->display_info
> to check if the connected display is HDMI 2.0.
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jose Abreu <jose.abreu@synopsys.com>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
>
> PS: This patch touches a few lines in few files, which were
> already above 80 char, so checkpatch gives 80 char warning again.
> - gpu/drm/omapdrm/omap_encoder.c
> - gpu/drm/i915/intel_sdvo.c
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
> drivers/gpu/drm/tegra/hdmi.c | 2 +-
> drivers/gpu/drm/tegra/sor.c | 2 +-
> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> include/drm/drm_edid.h | 3 ++-
> 21 files changed, 38 insertions(+), 21 deletions(-)
>
[snip]
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index af93f7a..5ff2886 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
> u8 val;
>
> /* Initialise info frame from DRM mode */
> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>
> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> frame.colorspace = HDMI_COLORSPACE_YUV444;
>
dw-hdmi controller has full support for HDMI 2.0 features. It all
depends on the platform it is integrated.
I think adding a parameter to
drm_hdmi_avi_infoframe_from_display_mode is not the best idea
because of this case: A bridge can have support for HDMI 2.0
features but the platform may limit this support. I guess it can
happen in other drivers too.
Best regards,
Jose Miguel Abreu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:22 ` Sharma, Shashank
@ 2017-03-23 15:28 ` Ilia Mirkin
2017-03-23 16:33 ` Sharma, Shashank
0 siblings, 1 reply; 22+ messages in thread
From: Ilia Mirkin @ 2017-03-23 15:28 UTC (permalink / raw)
To: Sharma, Shashank
Cc: Jose Abreu, Alex Deucher, Intel Graphics Development, dri-devel,
Daniel Vetter
On Thu, Mar 23, 2017 at 11:22 AM, Sharma, Shashank
<shashank.sharma@intel.com> wrote:
> On 3/23/2017 5:17 PM, Ilia Mirkin wrote:
>>
>> On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma
>> <shashank.sharma@intel.com> wrote:
>>>
>>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC
>>> 1-64).
>>> For any other mode, the VIC filed in AVI infoframes should be 0.
>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>>> extended to (VIC 1-107).
>>>
>>> This patch adds a bool input variable, which indicates if the connected
>>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>
>> Should this patch come before the patch which recognizes VICs 65+?
>> Otherwise if only the first patch is applied but not this one, you
>> could end up with the bad scenario. (As can happen in bisections, for
>> example.)
>
> I kindof agree, this could be the case, but also, for the correct
> functionality, you should have the whole series
> together.
OK, so ... I have a bug in my filesystem, and I'm bisecting it and
rebooting my box at each step. I happen to hit this commit in my
bisect and my monitor doesn't turn on? Seems unpleasant, no?
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:14 ` [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames Shashank Sharma
2017-03-23 15:17 ` Ilia Mirkin
2017-03-23 15:28 ` Jose Abreu
@ 2017-03-23 15:36 ` Ville Syrjälä
2017-03-23 15:52 ` Jose Abreu
2 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2017-03-23 15:36 UTC (permalink / raw)
To: Shashank Sharma
Cc: Jose Abreu, intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher,
Daniel Vetter
On Thu, Mar 23, 2017 at 05:14:19PM +0200, Shashank Sharma wrote:
> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
> For any other mode, the VIC filed in AVI infoframes should be 0.
> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
> extended to (VIC 1-107).
>
> This patch adds a bool input variable, which indicates if the connected
> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
> HDMI 2.0 VIC to a HDMI 1.4 sink.
The spec is unfortunately vague when it comes to the CEA-861-F VIC
transmission when there is a corresponding HDMI VIC for the same mode.
I'm not sure if it's telling us to set both or just one depending on
whether we're transmitting 3D video or not. Or at least I can't parse
that information from the spec. Anyone have a better crystal ball
in their possession?
>
> This patch touches all drm drivers, who are callers of this function
> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
> no change in current behavior, is_hdmi2 is kept as false.
>
> In case of I915 driver, this patch checks the connector->display_info
> to check if the connected display is HDMI 2.0.
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jose Abreu <jose.abreu@synopsys.com>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
>
> PS: This patch touches a few lines in few files, which were
> already above 80 char, so checkpatch gives 80 char warning again.
> - gpu/drm/omapdrm/omap_encoder.c
> - gpu/drm/i915/intel_sdvo.c
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
> drivers/gpu/drm/tegra/hdmi.c | 2 +-
> drivers/gpu/drm/tegra/sor.c | 2 +-
> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> include/drm/drm_edid.h | 3 ++-
> 21 files changed, 38 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index d4452d8..ab7a399 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
> dce_v10_0_audio_write_sad_regs(encoder);
> dce_v10_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 5b24e89..3c5fd83 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
> dce_v11_0_audio_write_sad_regs(encoder);
> dce_v11_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index d2590d7..c564dca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
> dce_v8_0_audio_write_sad_regs(encoder);
> dce_v8_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> index a2a8236..f9b77b8 100644
> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> @@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
>
> mutex_lock(&anx78xx->lock);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
> + false);
> if (err) {
> DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
> goto unlock;
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 9126d03..dcf15d7 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -269,7 +269,7 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> if (ret)
> return;
>
> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return;
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index af93f7a..5ff2886 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
> u8 val;
>
> /* Initialise info frame from DRM mode */
> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>
> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> frame.colorspace = HDMI_COLORSPACE_YUV444;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3b494c2..f9a4b08 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4664,12 +4664,14 @@ EXPORT_SYMBOL(drm_set_preferred_mode);
> * data from a DRM display mode
> * @frame: HDMI AVI infoframe
> * @mode: DRM display mode
> + * @is_hdmi2: Sink is HDMI 2.0 compliant
> *
> * Return: 0 on success or a negative error code on failure.
> */
> int
> drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> - const struct drm_display_mode *mode)
> + const struct drm_display_mode *mode,
> + bool is_hdmi2)
> {
> int err;
>
> @@ -4685,6 +4687,14 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>
> frame->video_code = drm_match_cea_mode(mode);
>
> + /*
> + * HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
> + * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
> + * have to make sure we dont break HDMI 1.4 sinks.
> + */
> + if (!is_hdmi2 && frame->video_code > 64)
> + frame->video_code = 0;
> +
> frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
>
> /*
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 5243840..5cb44d4 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -781,7 +781,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
> }
>
> ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> - &hdata->current_mode);
> + &hdata->current_mode, false);
> if (!ret)
> ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
> if (ret > 0) {
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 86f47e1..d1e7ac5 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -712,7 +712,7 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
> {
> union hdmi_infoframe frame;
>
> - drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> + drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
>
> tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 3eec74c..96644f3 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -458,11 +458,14 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> const struct drm_display_mode *adjusted_mode =
> &crtc_state->base.adjusted_mode;
> + struct drm_connector *connector = &intel_hdmi->attached_connector->base;
> + bool is_hdmi2 = connector->display_info.hdmi.scdc.supported;
> union hdmi_infoframe frame;
> int ret;
>
> ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> - adjusted_mode);
> + adjusted_mode,
> + is_hdmi2);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return;
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 816a6f5..694f626 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1011,7 +1011,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> ssize_t len;
>
> ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> - &pipe_config->base.adjusted_mode);
> + &pipe_config->base.adjusted_mode,
> + false);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return false;
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index c262512..80c36af 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -975,7 +975,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
> u8 buffer[17];
> ssize_t err;
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> dev_err(hdmi->dev,
> "Failed to get AVI infoframe from mode: %zd\n", err);
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 86c977b..624f5b5 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -85,7 +85,8 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
> if (hdmi_mode && dssdev->driver->set_hdmi_infoframe) {
> struct hdmi_avi_infoframe avi;
>
> - r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode);
> + r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
> + false);
> if (r == 0)
> dssdev->driver->set_hdmi_infoframe(dssdev, &avi);
> }
> diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
> index b214663..49b42bf 100644
> --- a/drivers/gpu/drm/radeon/radeon_audio.c
> +++ b/drivers/gpu/drm/radeon/radeon_audio.c
> @@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
> if (!connector)
> return -EINVAL;
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %d\n", err);
> return err;
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 006260d..b105f1c 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -294,7 +294,7 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
> union hdmi_infoframe frame;
> int rc;
>
> - rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> + rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
>
> if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
> frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index ce2dcba..2128b8c 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -434,7 +434,7 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
>
> DRM_DEBUG_DRIVER("\n");
>
> - ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode);
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false);
> if (ret < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %d\n", ret);
> return ret;
> diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
> index cda0491..718d8db 100644
> --- a/drivers/gpu/drm/tegra/hdmi.c
> +++ b/drivers/gpu/drm/tegra/hdmi.c
> @@ -734,7 +734,7 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
> u8 buffer[17];
> ssize_t err;
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index a8f5289..fb2709c 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -1904,7 +1904,7 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
> value &= ~INFOFRAME_CTRL_ENABLE;
> tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
> return err;
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index e9cbe26..5d81301 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -394,7 +394,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
> union hdmi_infoframe frame;
> int ret;
>
> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> if (ret < 0) {
> DRM_ERROR("couldn't fill AVI infoframe\n");
> return;
> diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
> index c47b9cb..3e267dd 100644
> --- a/drivers/gpu/drm/zte/zx_hdmi.c
> +++ b/drivers/gpu/drm/zte/zx_hdmi.c
> @@ -125,7 +125,7 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
> union hdmi_infoframe frame;
> int ret;
>
> - ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode);
> + ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> if (ret) {
> DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n",
> ret);
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index a21d494..a4d174e7 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -348,7 +348,8 @@ drm_load_edid_firmware(struct drm_connector *connector)
>
> int
> drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> - const struct drm_display_mode *mode);
> + const struct drm_display_mode *mode,
> + bool is_hdmi2);
> int
> drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
> const struct drm_display_mode *mode);
> --
> 2.7.4
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:28 ` Jose Abreu
@ 2017-03-23 15:45 ` Ville Syrjälä
2017-03-23 15:57 ` Jose Abreu
0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2017-03-23 15:45 UTC (permalink / raw)
To: Jose Abreu
Cc: intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher, Daniel Vetter
On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
> Hi Shashank,
>
>
> On 23-03-2017 15:14, Shashank Sharma wrote:
> > HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
> > For any other mode, the VIC filed in AVI infoframes should be 0.
> > HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
> > extended to (VIC 1-107).
> >
> > This patch adds a bool input variable, which indicates if the connected
> > sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
> > HDMI 2.0 VIC to a HDMI 1.4 sink.
> >
> > This patch touches all drm drivers, who are callers of this function
> > drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
> > no change in current behavior, is_hdmi2 is kept as false.
> >
> > In case of I915 driver, this patch checks the connector->display_info
> > to check if the connected display is HDMI 2.0.
> >
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Jose Abreu <jose.abreu@synopsys.com>
> > Cc: Andrzej Hajda <a.hajda@samsung.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> >
> > PS: This patch touches a few lines in few files, which were
> > already above 80 char, so checkpatch gives 80 char warning again.
> > - gpu/drm/omapdrm/omap_encoder.c
> > - gpu/drm/i915/intel_sdvo.c
> >
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> > drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
> > drivers/gpu/drm/bridge/sii902x.c | 2 +-
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> > drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
> > drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> > drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> > drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
> > drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
> > drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> > drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
> > drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
> > drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> > drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
> > drivers/gpu/drm/tegra/hdmi.c | 2 +-
> > drivers/gpu/drm/tegra/sor.c | 2 +-
> > drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
> > drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> > include/drm/drm_edid.h | 3 ++-
> > 21 files changed, 38 insertions(+), 21 deletions(-)
> >
>
> [snip]
>
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index af93f7a..5ff2886 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
> > u8 val;
> >
> > /* Initialise info frame from DRM mode */
> > - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> > + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> >
> > if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> > frame.colorspace = HDMI_COLORSPACE_YUV444;
> >
>
> dw-hdmi controller has full support for HDMI 2.0 features. It all
> depends on the platform it is integrated.
>
> I think adding a parameter to
> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
> because of this case: A bridge can have support for HDMI 2.0
> features but the platform may limit this support. I guess it can
> happen in other drivers too.
Your driver is in full control of what gets passed here. So I don't see
why that would be a problem.
Also this doesn't really have anything to do with the capabilities of
the source. All we want to make sure is that we don't send a VIC the
sink will not understand.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:36 ` Ville Syrjälä
@ 2017-03-23 15:52 ` Jose Abreu
2017-03-23 16:31 ` Sharma, Shashank
0 siblings, 1 reply; 22+ messages in thread
From: Jose Abreu @ 2017-03-23 15:52 UTC (permalink / raw)
To: Ville Syrjälä, Shashank Sharma
Cc: Jose Abreu, intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher,
Daniel Vetter
Hi Ville,
On 23-03-2017 15:36, Ville Syrjälä wrote:
> On Thu, Mar 23, 2017 at 05:14:19PM +0200, Shashank Sharma wrote:
>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
>> For any other mode, the VIC filed in AVI infoframes should be 0.
>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>> extended to (VIC 1-107).
>>
>> This patch adds a bool input variable, which indicates if the connected
>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>> HDMI 2.0 VIC to a HDMI 1.4 sink.
> The spec is unfortunately vague when it comes to the CEA-861-F VIC
> transmission when there is a corresponding HDMI VIC for the same mode.
> I'm not sure if it's telling us to set both or just one depending on
> whether we're transmitting 3D video or not. Or at least I can't parse
> that information from the spec. Anyone have a better crystal ball
> in their possession?
>
I've been working in HDMI receivers and this is what I've got in
a comment:
1282
/*
1283 * Update current VIC: When transmitting any
extended video format
1284 * indicated through use of the HDMI_VIC field in
the HDMI Vendor
1285 * Specific InfoFrame or any other format which is
not described in
1286 * the above cases, an HDMI Source shall set the AVI
InfoFrame VIC
1287 * field to
zero.
1288 */
This was directly taken from the spec, can't remember exactly
were though.
So, the VIC in AVIIF must be set to 0 and the HDMI_VIC field in
VSIF shall be set to the HDMI 4k VIC.
Best regards,
Jose Miguel Abreu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:45 ` Ville Syrjälä
@ 2017-03-23 15:57 ` Jose Abreu
2017-03-23 16:08 ` Sharma, Shashank
0 siblings, 1 reply; 22+ messages in thread
From: Jose Abreu @ 2017-03-23 15:57 UTC (permalink / raw)
To: Ville Syrjälä, Jose Abreu
Cc: intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher, Daniel Vetter
Hi Ville,
On 23-03-2017 15:45, Ville Syrjälä wrote:
> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
>> Hi Shashank,
>>
>>
>> On 23-03-2017 15:14, Shashank Sharma wrote:
>>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
>>> For any other mode, the VIC filed in AVI infoframes should be 0.
>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>>> extended to (VIC 1-107).
>>>
>>> This patch adds a bool input variable, which indicates if the connected
>>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>>
>>> This patch touches all drm drivers, who are callers of this function
>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
>>> no change in current behavior, is_hdmi2 is kept as false.
>>>
>>> In case of I915 driver, this patch checks the connector->display_info
>>> to check if the connected display is HDMI 2.0.
>>>
>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>
>>> PS: This patch touches a few lines in few files, which were
>>> already above 80 char, so checkpatch gives 80 char warning again.
>>> - gpu/drm/omapdrm/omap_encoder.c
>>> - gpu/drm/i915/intel_sdvo.c
>>>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>> drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
>>> drivers/gpu/drm/tegra/sor.c | 2 +-
>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
>>> include/drm/drm_edid.h | 3 ++-
>>> 21 files changed, 38 insertions(+), 21 deletions(-)
>>>
>> [snip]
>>
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index af93f7a..5ff2886 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>>> u8 val;
>>>
>>> /* Initialise info frame from DRM mode */
>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>>>
>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>
>> dw-hdmi controller has full support for HDMI 2.0 features. It all
>> depends on the platform it is integrated.
>>
>> I think adding a parameter to
>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
>> because of this case: A bridge can have support for HDMI 2.0
>> features but the platform may limit this support. I guess it can
>> happen in other drivers too.
> Your driver is in full control of what gets passed here. So I don't see
> why that would be a problem.
>
> Also this doesn't really have anything to do with the capabilities of
> the source. All we want to make sure is that we don't send a VIC the
> sink will not understand.
>
But the driver is not aware of the platform limitations, its
generic to the controller only. We could add a field in pdata
which tells if platform is HDMI 2.0+ but what about other bridge
drivers or HDMI drivers? They will have to replicate the same
thing also.
Best regards,
Jose Miguel Abreu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v4,1/2] drm/edid: Complete CEA modedb(VIC 1-107)
2017-03-23 15:14 [PATCH v4 1/2] drm/edid: Complete CEA modedb(VIC 1-107) Shashank Sharma
2017-03-23 15:14 ` [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames Shashank Sharma
@ 2017-03-23 16:01 ` Patchwork
1 sibling, 0 replies; 22+ messages in thread
From: Patchwork @ 2017-03-23 16:01 UTC (permalink / raw)
To: Shashank Sharma; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v4,1/2] drm/edid: Complete CEA modedb(VIC 1-107)
URL : https://patchwork.freedesktop.org/series/21772/
State : success
== Summary ==
Series 21772v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/21772/revisions/1/mbox/
Test gem_sync:
Subgroup basic-many-each:
incomplete -> PASS (fi-ilk-650)
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 463s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 452s
fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 584s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 536s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 578s
fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 510s
fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 504s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 442s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 518s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 501s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 485s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 485s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 606s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 491s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 520s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 461s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 553s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 415s
0b29bac4adfc0329c9235f8e496eaefb2bcc41e9 drm-tip: 2017y-03m-23d-14h-20m-03s UTC integration manifest
3aebd5f drm: Add HDMI 2.0 VIC support for AVI info-frames
7143d26 drm/edid: Complete CEA modedb(VIC 1-107)
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4281/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:57 ` Jose Abreu
@ 2017-03-23 16:08 ` Sharma, Shashank
2017-03-23 16:33 ` Jose Abreu
0 siblings, 1 reply; 22+ messages in thread
From: Sharma, Shashank @ 2017-03-23 16:08 UTC (permalink / raw)
To: Jose Abreu, Ville Syrjälä
Cc: Alex Deucher, intel-gfx, dri-devel, Daniel Vetter
Regards
Shashank
On 3/23/2017 5:57 PM, Jose Abreu wrote:
> Hi Ville,
>
>
> On 23-03-2017 15:45, Ville Syrjälä wrote:
>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
>>> Hi Shashank,
>>>
>>>
>>> On 23-03-2017 15:14, Shashank Sharma wrote:
>>>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
>>>> For any other mode, the VIC filed in AVI infoframes should be 0.
>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>>>> extended to (VIC 1-107).
>>>>
>>>> This patch adds a bool input variable, which indicates if the connected
>>>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>>>
>>>> This patch touches all drm drivers, who are callers of this function
>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
>>>> no change in current behavior, is_hdmi2 is kept as false.
>>>>
>>>> In case of I915 driver, this patch checks the connector->display_info
>>>> to check if the connected display is HDMI 2.0.
>>>>
>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>>
>>>> PS: This patch touches a few lines in few files, which were
>>>> already above 80 char, so checkpatch gives 80 char warning again.
>>>> - gpu/drm/omapdrm/omap_encoder.c
>>>> - gpu/drm/i915/intel_sdvo.c
>>>>
>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
>>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
>>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
>>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
>>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>>> drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
>>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
>>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
>>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
>>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
>>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
>>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
>>>> drivers/gpu/drm/tegra/sor.c | 2 +-
>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
>>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
>>>> include/drm/drm_edid.h | 3 ++-
>>>> 21 files changed, 38 insertions(+), 21 deletions(-)
>>>>
>>> [snip]
>>>
>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> index af93f7a..5ff2886 100644
>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>> u8 val;
>>>>
>>>> /* Initialise info frame from DRM mode */
>>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>>>>
>>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>
>>> dw-hdmi controller has full support for HDMI 2.0 features. It all
>>> depends on the platform it is integrated.
>>>
>>> I think adding a parameter to
>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
>>> because of this case: A bridge can have support for HDMI 2.0
>>> features but the platform may limit this support. I guess it can
>>> happen in other drivers too.
>> Your driver is in full control of what gets passed here. So I don't see
>> why that would be a problem.
>>
>> Also this doesn't really have anything to do with the capabilities of
>> the source. All we want to make sure is that we don't send a VIC the
>> sink will not understand.
>>
> But the driver is not aware of the platform limitations, its
> generic to the controller only. We could add a field in pdata
> which tells if platform is HDMI 2.0+ but what about other bridge
> drivers or HDMI drivers? They will have to replicate the same
> thing also.
>
> Best regards,
> Jose Miguel Abreu
I think the driver would be aware of the platform's capabilities, isn't
it ?
Else how would it even decide which mode to allow, and which to reject ?
Regards
Shashank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:52 ` Jose Abreu
@ 2017-03-23 16:31 ` Sharma, Shashank
2017-03-23 17:01 ` Ville Syrjälä
0 siblings, 1 reply; 22+ messages in thread
From: Sharma, Shashank @ 2017-03-23 16:31 UTC (permalink / raw)
To: Jose Abreu, Ville Syrjälä
Cc: Alex Deucher, intel-gfx, dri-devel, Daniel Vetter
Regards
Shashank
On 3/23/2017 5:52 PM, Jose Abreu wrote:
> Hi Ville,
>
>
> On 23-03-2017 15:36, Ville Syrjälä wrote:
>> On Thu, Mar 23, 2017 at 05:14:19PM +0200, Shashank Sharma wrote:
>>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
>>> For any other mode, the VIC filed in AVI infoframes should be 0.
>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>>> extended to (VIC 1-107).
>>>
>>> This patch adds a bool input variable, which indicates if the connected
>>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>> The spec is unfortunately vague when it comes to the CEA-861-F VIC
>> transmission when there is a corresponding HDMI VIC for the same mode.
>> I'm not sure if it's telling us to set both or just one depending on
>> whether we're transmitting 3D video or not. Or at least I can't parse
>> that information from the spec. Anyone have a better crystal ball
>> in their possession?
>>
> I've been working in HDMI receivers and this is what I've got in
> a comment:
>
> 1282
> /*
>
> 1283 * Update current VIC: When transmitting any
> extended video format
> 1284 * indicated through use of the HDMI_VIC field in
> the HDMI Vendor
> 1285 * Specific InfoFrame or any other format which is
> not described in
> 1286 * the above cases, an HDMI Source shall set the AVI
> InfoFrame VIC
> 1287 * field to
> zero.
>
> 1288 */
>
> This was directly taken from the spec, can't remember exactly
> were though.
>
> So, the VIC in AVIIF must be set to 0 and the HDMI_VIC field in
> VSIF shall be set to the HDMI 4k VIC.
>
> Best regards,
> Jose Miguel Abreu
Even my understanding of the two specs seems similar
If its a HDMI 1.4b monitor, 2D mode
- VIC field in AVI_IF should be set to appropriate VIC [if VIC is
not listed in CEA-861-D (VIC 1-64), make VIC=0]
If its a HDMI 1.4b monitor, 3D mode
- VIC field in AVI_IF should be set to appropriate VIC, in
conjunction with 3D_structure field in HDMI VSIF
If its a HDMI 2.0 monitor, 2D mode
- VIC filed in the AVI_IF should be set to appropriate VIC (1-107)
If its a HDMI 2.0 monitor, 3D mode
- VIC field in AVI_IF should be set to appropriate VIC, in
conjunction with 3D_structure field in HDMI VSIF
Regards
Shashank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 15:28 ` Ilia Mirkin
@ 2017-03-23 16:33 ` Sharma, Shashank
0 siblings, 0 replies; 22+ messages in thread
From: Sharma, Shashank @ 2017-03-23 16:33 UTC (permalink / raw)
To: Ilia Mirkin
Cc: Jose Abreu, Alex Deucher, Intel Graphics Development, dri-devel,
Daniel Vetter
Regards
Shashank
On 3/23/2017 5:28 PM, Ilia Mirkin wrote:
> On Thu, Mar 23, 2017 at 11:22 AM, Sharma, Shashank
> <shashank.sharma@intel.com> wrote:
>> On 3/23/2017 5:17 PM, Ilia Mirkin wrote:
>>> On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma
>>> <shashank.sharma@intel.com> wrote:
>>>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC
>>>> 1-64).
>>>> For any other mode, the VIC filed in AVI infoframes should be 0.
>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>>>> extended to (VIC 1-107).
>>>>
>>>> This patch adds a bool input variable, which indicates if the connected
>>>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>> Should this patch come before the patch which recognizes VICs 65+?
>>> Otherwise if only the first patch is applied but not this one, you
>>> could end up with the bad scenario. (As can happen in bisections, for
>>> example.)
>> I kindof agree, this could be the case, but also, for the correct
>> functionality, you should have the whole series
>> together.
> OK, so ... I have a bug in my filesystem, and I'm bisecting it and
> rebooting my box at each step. I happen to hit this commit in my
> bisect and my monitor doesn't turn on? Seems unpleasant, no?
Cant say no :-), will happily change the order if this makes you feel
better.
Regards
Shashank
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 16:08 ` Sharma, Shashank
@ 2017-03-23 16:33 ` Jose Abreu
2017-03-23 16:43 ` Sharma, Shashank
0 siblings, 1 reply; 22+ messages in thread
From: Jose Abreu @ 2017-03-23 16:33 UTC (permalink / raw)
To: Sharma, Shashank, Jose Abreu, Ville Syrjälä
Cc: Alex Deucher, Andrzej Hajda, intel-gfx, dri-devel, Daniel Vetter
Hi Shashank,
On 23-03-2017 16:08, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 3/23/2017 5:57 PM, Jose Abreu wrote:
>> Hi Ville,
>>
>>
>> On 23-03-2017 15:45, Ville Syrjälä wrote:
>>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
>>>> Hi Shashank,
>>>>
>>>>
>>>> On 23-03-2017 15:14, Shashank Sharma wrote:
>>>>> HDMI 1.4b support the CEA video modes as per range of
>>>>> CEA-861-D (VIC 1-64).
>>>>> For any other mode, the VIC filed in AVI infoframes should
>>>>> be 0.
>>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F
>>>>> spec, which is
>>>>> extended to (VIC 1-107).
>>>>>
>>>>> This patch adds a bool input variable, which indicates if
>>>>> the connected
>>>>> sink is a HDMI 2.0 sink or not. This will make sure that we
>>>>> don't pass a
>>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>>>>
>>>>> This patch touches all drm drivers, who are callers of this
>>>>> function
>>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure
>>>>> there is
>>>>> no change in current behavior, is_hdmi2 is kept as false.
>>>>>
>>>>> In case of I915 driver, this patch checks the
>>>>> connector->display_info
>>>>> to check if the connected display is HDMI 2.0.
>>>>>
>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>>>
>>>>> PS: This patch touches a few lines in few files, which were
>>>>> already above 80 char, so checkpatch gives 80 char warning
>>>>> again.
>>>>> - gpu/drm/omapdrm/omap_encoder.c
>>>>> - gpu/drm/i915/intel_sdvo.c
>>>>>
>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
>>>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
>>>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
>>>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
>>>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>>>> drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
>>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
>>>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>>>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
>>>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
>>>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
>>>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
>>>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>>>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
>>>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
>>>>> drivers/gpu/drm/tegra/sor.c | 2 +-
>>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
>>>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
>>>>> include/drm/drm_edid.h | 3 ++-
>>>>> 21 files changed, 38 insertions(+), 21 deletions(-)
>>>>>
>>>> [snip]
>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> index af93f7a..5ff2886 100644
>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct
>>>>> dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>>> u8 val;
>>>>> /* Initialise info frame from DRM mode */
>>>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode,
>>>>> false);
>>>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>>
>>>> dw-hdmi controller has full support for HDMI 2.0 features.
>>>> It all
>>>> depends on the platform it is integrated.
>>>>
>>>> I think adding a parameter to
>>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
>>>> because of this case: A bridge can have support for HDMI 2.0
>>>> features but the platform may limit this support. I guess it
>>>> can
>>>> happen in other drivers too.
>>> Your driver is in full control of what gets passed here. So I
>>> don't see
>>> why that would be a problem.
>>>
>>> Also this doesn't really have anything to do with the
>>> capabilities of
>>> the source. All we want to make sure is that we don't send a
>>> VIC the
>>> sink will not understand.
>>>
>> But the driver is not aware of the platform limitations, its
>> generic to the controller only. We could add a field in pdata
>> which tells if platform is HDMI 2.0+ but what about other bridge
>> drivers or HDMI drivers? They will have to replicate the same
>> thing also.
>>
>> Best regards,
>> Jose Miguel Abreu
> I think the driver would be aware of the platform's
> capabilities, isn't it ?
> Else how would it even decide which mode to allow, and which to
> reject ?
The DRM core propagates the mode to the chain of configuration
before reaching the bridge driver also, there is a callback
supplied by pdata (mode_valid) which can check if the mode is
valid. (see
https://cgit.freedesktop.org/~airlied/linux/tree/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c?h=drm-next#n1740)
Best regards,
Jose Miguel Abreu
>
> Regards
> Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 16:33 ` Jose Abreu
@ 2017-03-23 16:43 ` Sharma, Shashank
2017-03-23 17:11 ` Jose Abreu
0 siblings, 1 reply; 22+ messages in thread
From: Sharma, Shashank @ 2017-03-23 16:43 UTC (permalink / raw)
To: Jose Abreu, Ville Syrjälä
Cc: Alex Deucher, Andrzej Hajda, intel-gfx, dri-devel, Daniel Vetter
Regards
Shashank
On 3/23/2017 6:33 PM, Jose Abreu wrote:
> Hi Shashank,
>
>
> On 23-03-2017 16:08, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 3/23/2017 5:57 PM, Jose Abreu wrote:
>>> Hi Ville,
>>>
>>>
>>> On 23-03-2017 15:45, Ville Syrjälä wrote:
>>>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
>>>>> Hi Shashank,
>>>>>
>>>>>
>>>>> On 23-03-2017 15:14, Shashank Sharma wrote:
>>>>>> HDMI 1.4b support the CEA video modes as per range of
>>>>>> CEA-861-D (VIC 1-64).
>>>>>> For any other mode, the VIC filed in AVI infoframes should
>>>>>> be 0.
>>>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F
>>>>>> spec, which is
>>>>>> extended to (VIC 1-107).
>>>>>>
>>>>>> This patch adds a bool input variable, which indicates if
>>>>>> the connected
>>>>>> sink is a HDMI 2.0 sink or not. This will make sure that we
>>>>>> don't pass a
>>>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>>>>>
>>>>>> This patch touches all drm drivers, who are callers of this
>>>>>> function
>>>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure
>>>>>> there is
>>>>>> no change in current behavior, is_hdmi2 is kept as false.
>>>>>>
>>>>>> In case of I915 driver, this patch checks the
>>>>>> connector->display_info
>>>>>> to check if the connected display is HDMI 2.0.
>>>>>>
>>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>>>>
>>>>>> PS: This patch touches a few lines in few files, which were
>>>>>> already above 80 char, so checkpatch gives 80 char warning
>>>>>> again.
>>>>>> - gpu/drm/omapdrm/omap_encoder.c
>>>>>> - gpu/drm/i915/intel_sdvo.c
>>>>>>
>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
>>>>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
>>>>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>>>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>>>>> drivers/gpu/drm/drm_edid.c | 12 +++++++++++-
>>>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
>>>>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>>>>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
>>>>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
>>>>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
>>>>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
>>>>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>>>>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
>>>>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
>>>>>> drivers/gpu/drm/tegra/sor.c | 2 +-
>>>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
>>>>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
>>>>>> include/drm/drm_edid.h | 3 ++-
>>>>>> 21 files changed, 38 insertions(+), 21 deletions(-)
>>>>>>
>>>>> [snip]
>>>>>
>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>> index af93f7a..5ff2886 100644
>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct
>>>>>> dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>>>> u8 val;
>>>>>> /* Initialise info frame from DRM mode */
>>>>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>>>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode,
>>>>>> false);
>>>>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>>>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>>>
>>>>> dw-hdmi controller has full support for HDMI 2.0 features.
>>>>> It all
>>>>> depends on the platform it is integrated.
>>>>>
>>>>> I think adding a parameter to
>>>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
>>>>> because of this case: A bridge can have support for HDMI 2.0
>>>>> features but the platform may limit this support. I guess it
>>>>> can
>>>>> happen in other drivers too.
>>>> Your driver is in full control of what gets passed here. So I
>>>> don't see
>>>> why that would be a problem.
>>>>
>>>> Also this doesn't really have anything to do with the
>>>> capabilities of
>>>> the source. All we want to make sure is that we don't send a
>>>> VIC the
>>>> sink will not understand.
>>>>
>>> But the driver is not aware of the platform limitations, its
>>> generic to the controller only. We could add a field in pdata
>>> which tells if platform is HDMI 2.0+ but what about other bridge
>>> drivers or HDMI drivers? They will have to replicate the same
>>> thing also.
>>>
>>> Best regards,
>>> Jose Miguel Abreu
>> I think the driver would be aware of the platform's
>> capabilities, isn't it ?
>> Else how would it even decide which mode to allow, and which to
>> reject ?
> The DRM core propagates the mode to the chain of configuration
> before reaching the bridge driver also, there is a callback
> supplied by pdata (mode_valid) which can check if the mode is
> valid. (see
> https://cgit.freedesktop.org/~airlied/linux/tree/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c?h=drm-next#n1740)
>
> Best regards,
> Jose Miguel Abreu
>
>
>> Regards
>> Shashank
Please correct me if my understanding is not right, but drivers call
mode_valid() to prune/reject modes which they cant support.
and they call drm_set_infoframe_from_videomode() function, when they are
going ahead to the modeset with a mode.
Why would a driver choose to do a modeset, which it could not support
(shouldn't mode_valid have dropped it in atomic_check() or may be even
edid_parsing time ?)
Regards
Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 16:31 ` Sharma, Shashank
@ 2017-03-23 17:01 ` Ville Syrjälä
0 siblings, 0 replies; 22+ messages in thread
From: Ville Syrjälä @ 2017-03-23 17:01 UTC (permalink / raw)
To: Sharma, Shashank
Cc: Jose Abreu, intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher,
Daniel Vetter
On Thu, Mar 23, 2017 at 06:31:33PM +0200, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 3/23/2017 5:52 PM, Jose Abreu wrote:
> > Hi Ville,
> >
> >
> > On 23-03-2017 15:36, Ville Syrjälä wrote:
> >> On Thu, Mar 23, 2017 at 05:14:19PM +0200, Shashank Sharma wrote:
> >>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
> >>> For any other mode, the VIC filed in AVI infoframes should be 0.
> >>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
> >>> extended to (VIC 1-107).
> >>>
> >>> This patch adds a bool input variable, which indicates if the connected
> >>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
> >>> HDMI 2.0 VIC to a HDMI 1.4 sink.
> >> The spec is unfortunately vague when it comes to the CEA-861-F VIC
> >> transmission when there is a corresponding HDMI VIC for the same mode.
> >> I'm not sure if it's telling us to set both or just one depending on
> >> whether we're transmitting 3D video or not. Or at least I can't parse
> >> that information from the spec. Anyone have a better crystal ball
> >> in their possession?
> >>
> > I've been working in HDMI receivers and this is what I've got in
> > a comment:
> >
> > 1282
> > /*
> >
> > 1283 * Update current VIC: When transmitting any
> > extended video format
> > 1284 * indicated through use of the HDMI_VIC field in
> > the HDMI Vendor
> > 1285 * Specific InfoFrame or any other format which is
> > not described in
> > 1286 * the above cases, an HDMI Source shall set the AVI
> > InfoFrame VIC
> > 1287 * field to
> > zero.
> >
> > 1288 */
> >
> > This was directly taken from the spec, can't remember exactly
> > were though.
> >
> > So, the VIC in AVIIF must be set to 0 and the HDMI_VIC field in
> > VSIF shall be set to the HDMI 4k VIC.
> >
> > Best regards,
> > Jose Miguel Abreu
> Even my understanding of the two specs seems similar
> If its a HDMI 1.4b monitor, 2D mode
> - VIC field in AVI_IF should be set to appropriate VIC [if VIC is
> not listed in CEA-861-D (VIC 1-64), make VIC=0]
> If its a HDMI 1.4b monitor, 3D mode
> - VIC field in AVI_IF should be set to appropriate VIC, in
> conjunction with 3D_structure field in HDMI VSIF
> If its a HDMI 2.0 monitor, 2D mode
> - VIC filed in the AVI_IF should be set to appropriate VIC (1-107)
> If its a HDMI 2.0 monitor, 3D mode
> - VIC field in AVI_IF should be set to appropriate VIC, in
> conjunction with 3D_structure field in HDMI VSIF
You're overlooking the HDMI VIC entirely in that list.
OK, so our code even refuses to send both 3D stuff and HDMI VIC
at the same time. I presume this was an illegal combination in HDMI 1.4.
And since there was no overlap between the CEA VICs and HDMI VICs this
was all that we needed.
But now that there is overlap, I think we'll need to set CEA VIC=0
whenever HDMI VIC!=0. Or at least that's my strictest interpretation of
the spec. I wish they would have stated this stuff more explicitly.
So either we need to pass the AVI IF to
drm_hdmi_vendor_infoframe_from_display_mode() and have it clear the AVI
IF VIC if HDMI VIC is specified, or we need to pass the HDMI IF to
drm_hdmi_avi_infoframe_from_display_mode() and have it not set the AVI
IF VIC if HDMI VIC is specified. Or we need to make the caller handle
this, which would probably lead to more bugs.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 16:43 ` Sharma, Shashank
@ 2017-03-23 17:11 ` Jose Abreu
2017-03-23 17:42 ` Ville Syrjälä
0 siblings, 1 reply; 22+ messages in thread
From: Jose Abreu @ 2017-03-23 17:11 UTC (permalink / raw)
To: Sharma, Shashank, Jose Abreu, Ville Syrjälä
Cc: Alex Deucher, Andrzej Hajda, intel-gfx, dri-devel, Daniel Vetter
Hi Shashank,
On 23-03-2017 16:43, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 3/23/2017 6:33 PM, Jose Abreu wrote:
>> Hi Shashank,
>>
>>
>> On 23-03-2017 16:08, Sharma, Shashank wrote:
>>> Regards
>>>
>>> Shashank
>>>
>>>
>>> On 3/23/2017 5:57 PM, Jose Abreu wrote:
>>>> Hi Ville,
>>>>
>>>>
>>>> On 23-03-2017 15:45, Ville Syrjälä wrote:
>>>>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
>>>>>> Hi Shashank,
>>>>>>
>>>>>>
>>>>>> On 23-03-2017 15:14, Shashank Sharma wrote:
>>>>>>> HDMI 1.4b support the CEA video modes as per range of
>>>>>>> CEA-861-D (VIC 1-64).
>>>>>>> For any other mode, the VIC filed in AVI infoframes should
>>>>>>> be 0.
>>>>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F
>>>>>>> spec, which is
>>>>>>> extended to (VIC 1-107).
>>>>>>>
>>>>>>> This patch adds a bool input variable, which indicates if
>>>>>>> the connected
>>>>>>> sink is a HDMI 2.0 sink or not. This will make sure that we
>>>>>>> don't pass a
>>>>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>>>>>>
>>>>>>> This patch touches all drm drivers, who are callers of this
>>>>>>> function
>>>>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure
>>>>>>> there is
>>>>>>> no change in current behavior, is_hdmi2 is kept as false.
>>>>>>>
>>>>>>> In case of I915 driver, this patch checks the
>>>>>>> connector->display_info
>>>>>>> to check if the connected display is HDMI 2.0.
>>>>>>>
>>>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>>>>>
>>>>>>> PS: This patch touches a few lines in few files, which were
>>>>>>> already above 80 char, so checkpatch gives 80 char warning
>>>>>>> again.
>>>>>>> - gpu/drm/omapdrm/omap_encoder.c
>>>>>>> - gpu/drm/i915/intel_sdvo.c
>>>>>>>
>>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
>>>>>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
>>>>>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>>>>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>>>>>> drivers/gpu/drm/drm_edid.c | 12
>>>>>>> +++++++++++-
>>>>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
>>>>>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>>>>>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
>>>>>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
>>>>>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
>>>>>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
>>>>>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>>>>>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
>>>>>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
>>>>>>> drivers/gpu/drm/tegra/sor.c | 2 +-
>>>>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
>>>>>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
>>>>>>> include/drm/drm_edid.h | 3 ++-
>>>>>>> 21 files changed, 38 insertions(+), 21 deletions(-)
>>>>>>>
>>>>>> [snip]
>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>> index af93f7a..5ff2886 100644
>>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct
>>>>>>> dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>>>>> u8 val;
>>>>>>> /* Initialise info frame from DRM mode */
>>>>>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>>>>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode,
>>>>>>> false);
>>>>>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>>>>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>>>>
>>>>>> dw-hdmi controller has full support for HDMI 2.0 features.
>>>>>> It all
>>>>>> depends on the platform it is integrated.
>>>>>>
>>>>>> I think adding a parameter to
>>>>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
>>>>>> because of this case: A bridge can have support for HDMI 2.0
>>>>>> features but the platform may limit this support. I guess it
>>>>>> can
>>>>>> happen in other drivers too.
>>>>> Your driver is in full control of what gets passed here. So I
>>>>> don't see
>>>>> why that would be a problem.
>>>>>
>>>>> Also this doesn't really have anything to do with the
>>>>> capabilities of
>>>>> the source. All we want to make sure is that we don't send a
>>>>> VIC the
>>>>> sink will not understand.
>>>>>
>>>> But the driver is not aware of the platform limitations, its
>>>> generic to the controller only. We could add a field in pdata
>>>> which tells if platform is HDMI 2.0+ but what about other
>>>> bridge
>>>> drivers or HDMI drivers? They will have to replicate the same
>>>> thing also.
>>>>
>>>> Best regards,
>>>> Jose Miguel Abreu
>>> I think the driver would be aware of the platform's
>>> capabilities, isn't it ?
>>> Else how would it even decide which mode to allow, and which to
>>> reject ?
>> The DRM core propagates the mode to the chain of configuration
>> before reaching the bridge driver also, there is a callback
>> supplied by pdata (mode_valid) which can check if the mode is
>> valid. (see
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_-7Eairlied_linux_tree_drivers_gpu_drm_bridge_synopsys_dw-2Dhdmi.c-3Fh-3Ddrm-2Dnext-23n1740&d=DwID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=zC1cWRy6RDWimQG2o866yYylV7c5h_U4aGWjVtnIHH0&s=DPZA3OgLWOO299hjy9dBDBK66kmUGfZArxKbeHNMKOc&e=
>> )
>>
>> Best regards,
>> Jose Miguel Abreu
>>
>>
>>> Regards
>>> Shashank
> Please correct me if my understanding is not right, but drivers
> call mode_valid() to prune/reject modes which they cant support.
> and they call drm_set_infoframe_from_videomode() function, when
> they are going ahead to the modeset with a mode.
> Why would a driver choose to do a modeset, which it could not
> support (shouldn't mode_valid have dropped it in atomic_check()
> or may be even edid_parsing time ?)
Yes, thats all correct :) I got a little out of topic here, sorry.
The thing I don't agree with is the adding of the parameter for
the drm_hdmi_avi_infoframe_from_display_mode(), because I think
its not enough. I will state why:
Picture this scenario:
1) EDID is read from HDMI 2.0 sink
2) The 2.0 modes are probed and as there is no check for
these modes (they are in the CEA table) they will be added to
mode list
3) The modes are passed to userspace. This is a problem
because userspace does not yet understand the new aspect ratios,
but lets imagine it understands
4) User selects an HDMI 2.0 mode
5) The drm core checks if mode is valid with driver, the
driver can say yes or no. Lets think it says yes because the
platform supports HDMI 2.0 modes.
6) Mode is committed but VIC will be set to zero according to
the patch
Now, according to 2.0 spec VIC shall be set when we are in 2.0
modes. I didn't do much inter-op with HDMI 2.0 receivers but I
can picture that some of them may just not display anything
because VIC and HDMI_VIC are not set. Though, from my experience,
generally the receiver is much more robust than the transmitter.
Even so, I think we should make sure that transmitter drivers are
fully compliant so that we can guarantee the support for less
robust receivers.
Best regards,
Jose Miguel Abreu
>
> Regards
> Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 17:11 ` Jose Abreu
@ 2017-03-23 17:42 ` Ville Syrjälä
2017-03-23 17:53 ` Jose Abreu
0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2017-03-23 17:42 UTC (permalink / raw)
To: Jose Abreu
Cc: intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher, Daniel Vetter
On Thu, Mar 23, 2017 at 05:11:44PM +0000, Jose Abreu wrote:
> Hi Shashank,
>
>
> On 23-03-2017 16:43, Sharma, Shashank wrote:
> > Regards
> >
> > Shashank
> >
> >
> > On 3/23/2017 6:33 PM, Jose Abreu wrote:
> >> Hi Shashank,
> >>
> >>
> >> On 23-03-2017 16:08, Sharma, Shashank wrote:
> >>> Regards
> >>>
> >>> Shashank
> >>>
> >>>
> >>> On 3/23/2017 5:57 PM, Jose Abreu wrote:
> >>>> Hi Ville,
> >>>>
> >>>>
> >>>> On 23-03-2017 15:45, Ville Syrjälä wrote:
> >>>>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
> >>>>>> Hi Shashank,
> >>>>>>
> >>>>>>
> >>>>>> On 23-03-2017 15:14, Shashank Sharma wrote:
> >>>>>>> HDMI 1.4b support the CEA video modes as per range of
> >>>>>>> CEA-861-D (VIC 1-64).
> >>>>>>> For any other mode, the VIC filed in AVI infoframes should
> >>>>>>> be 0.
> >>>>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F
> >>>>>>> spec, which is
> >>>>>>> extended to (VIC 1-107).
> >>>>>>>
> >>>>>>> This patch adds a bool input variable, which indicates if
> >>>>>>> the connected
> >>>>>>> sink is a HDMI 2.0 sink or not. This will make sure that we
> >>>>>>> don't pass a
> >>>>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
> >>>>>>>
> >>>>>>> This patch touches all drm drivers, who are callers of this
> >>>>>>> function
> >>>>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure
> >>>>>>> there is
> >>>>>>> no change in current behavior, is_hdmi2 is kept as false.
> >>>>>>>
> >>>>>>> In case of I915 driver, this patch checks the
> >>>>>>> connector->display_info
> >>>>>>> to check if the connected display is HDMI 2.0.
> >>>>>>>
> >>>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >>>>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
> >>>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >>>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
> >>>>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
> >>>>>>>
> >>>>>>> PS: This patch touches a few lines in few files, which were
> >>>>>>> already above 80 char, so checkpatch gives 80 char warning
> >>>>>>> again.
> >>>>>>> - gpu/drm/omapdrm/omap_encoder.c
> >>>>>>> - gpu/drm/i915/intel_sdvo.c
> >>>>>>>
> >>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>>>>> ---
> >>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
> >>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
> >>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> >>>>>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
> >>>>>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> >>>>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> >>>>>>> drivers/gpu/drm/drm_edid.c | 12
> >>>>>>> +++++++++++-
> >>>>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> >>>>>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> >>>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
> >>>>>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
> >>>>>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> >>>>>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
> >>>>>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
> >>>>>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> >>>>>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
> >>>>>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
> >>>>>>> drivers/gpu/drm/tegra/sor.c | 2 +-
> >>>>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
> >>>>>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> >>>>>>> include/drm/drm_edid.h | 3 ++-
> >>>>>>> 21 files changed, 38 insertions(+), 21 deletions(-)
> >>>>>>>
> >>>>>> [snip]
> >>>>>>
> >>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>> index af93f7a..5ff2886 100644
> >>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct
> >>>>>>> dw_hdmi *hdmi, struct drm_display_mode *mode)
> >>>>>>> u8 val;
> >>>>>>> /* Initialise info frame from DRM mode */
> >>>>>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> >>>>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode,
> >>>>>>> false);
> >>>>>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> >>>>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
> >>>>>>>
> >>>>>> dw-hdmi controller has full support for HDMI 2.0 features.
> >>>>>> It all
> >>>>>> depends on the platform it is integrated.
> >>>>>>
> >>>>>> I think adding a parameter to
> >>>>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
> >>>>>> because of this case: A bridge can have support for HDMI 2.0
> >>>>>> features but the platform may limit this support. I guess it
> >>>>>> can
> >>>>>> happen in other drivers too.
> >>>>> Your driver is in full control of what gets passed here. So I
> >>>>> don't see
> >>>>> why that would be a problem.
> >>>>>
> >>>>> Also this doesn't really have anything to do with the
> >>>>> capabilities of
> >>>>> the source. All we want to make sure is that we don't send a
> >>>>> VIC the
> >>>>> sink will not understand.
> >>>>>
> >>>> But the driver is not aware of the platform limitations, its
> >>>> generic to the controller only. We could add a field in pdata
> >>>> which tells if platform is HDMI 2.0+ but what about other
> >>>> bridge
> >>>> drivers or HDMI drivers? They will have to replicate the same
> >>>> thing also.
> >>>>
> >>>> Best regards,
> >>>> Jose Miguel Abreu
> >>> I think the driver would be aware of the platform's
> >>> capabilities, isn't it ?
> >>> Else how would it even decide which mode to allow, and which to
> >>> reject ?
> >> The DRM core propagates the mode to the chain of configuration
> >> before reaching the bridge driver also, there is a callback
> >> supplied by pdata (mode_valid) which can check if the mode is
> >> valid. (see
> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_-7Eairlied_linux_tree_drivers_gpu_drm_bridge_synopsys_dw-2Dhdmi.c-3Fh-3Ddrm-2Dnext-23n1740&d=DwID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=zC1cWRy6RDWimQG2o866yYylV7c5h_U4aGWjVtnIHH0&s=DPZA3OgLWOO299hjy9dBDBK66kmUGfZArxKbeHNMKOc&e=
> >> )
> >>
> >> Best regards,
> >> Jose Miguel Abreu
> >>
> >>
> >>> Regards
> >>> Shashank
> > Please correct me if my understanding is not right, but drivers
> > call mode_valid() to prune/reject modes which they cant support.
> > and they call drm_set_infoframe_from_videomode() function, when
> > they are going ahead to the modeset with a mode.
> > Why would a driver choose to do a modeset, which it could not
> > support (shouldn't mode_valid have dropped it in atomic_check()
> > or may be even edid_parsing time ?)
>
> Yes, thats all correct :) I got a little out of topic here, sorry.
>
> The thing I don't agree with is the adding of the parameter for
> the drm_hdmi_avi_infoframe_from_display_mode(), because I think
> its not enough. I will state why:
>
> Picture this scenario:
> 1) EDID is read from HDMI 2.0 sink
> 2) The 2.0 modes are probed and as there is no check for
> these modes (they are in the CEA table) they will be added to
> mode list
> 3) The modes are passed to userspace. This is a problem
> because userspace does not yet understand the new aspect ratios,
> but lets imagine it understands
> 4) User selects an HDMI 2.0 mode
> 5) The drm core checks if mode is valid with driver, the
> driver can say yes or no. Lets think it says yes because the
> platform supports HDMI 2.0 modes.
> 6) Mode is committed but VIC will be set to zero according to
> the patch
That's only because you haven't updated your driver to pass 'true'
as needed.
The other option would be to pass the connector instead of a boolean,
but according to Shashank that would required a lot of plumbing changes
because many drivers don't have the connector immediately available
where they call these functions. And plumbing the connector through
probably requires intimate knowledge of each driver, and thus is not
something we would be confortable doing. Hence I think the bool is the
best short term solution, letting each driver author figure out how to
get at the right connector and pass on the required information.
I don't really understand what you're even suggesting as an alternative.
We need to know what the sink supports, and if your driver is architected
in some crazy way where you don't even have that information where it's
needed, I don't think there's anything anyone can do.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 17:42 ` Ville Syrjälä
@ 2017-03-23 17:53 ` Jose Abreu
2017-03-23 18:14 ` Ville Syrjälä
0 siblings, 1 reply; 22+ messages in thread
From: Jose Abreu @ 2017-03-23 17:53 UTC (permalink / raw)
To: Ville Syrjälä, Jose Abreu
Cc: intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher, Daniel Vetter
Hi Ville,
On 23-03-2017 17:42, Ville Syrjälä wrote:
> On Thu, Mar 23, 2017 at 05:11:44PM +0000, Jose Abreu wrote:
>> Hi Shashank,
>>
>>
>> On 23-03-2017 16:43, Sharma, Shashank wrote:
>>> Regards
>>>
>>> Shashank
>>>
>>>
>>> On 3/23/2017 6:33 PM, Jose Abreu wrote:
>>>> Hi Shashank,
>>>>
>>>>
>>>> On 23-03-2017 16:08, Sharma, Shashank wrote:
>>>>> Regards
>>>>>
>>>>> Shashank
>>>>>
>>>>>
>>>>> On 3/23/2017 5:57 PM, Jose Abreu wrote:
>>>>>> Hi Ville,
>>>>>>
>>>>>>
>>>>>> On 23-03-2017 15:45, Ville Syrjälä wrote:
>>>>>>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
>>>>>>>> Hi Shashank,
>>>>>>>>
>>>>>>>>
>>>>>>>> On 23-03-2017 15:14, Shashank Sharma wrote:
>>>>>>>>> HDMI 1.4b support the CEA video modes as per range of
>>>>>>>>> CEA-861-D (VIC 1-64).
>>>>>>>>> For any other mode, the VIC filed in AVI infoframes should
>>>>>>>>> be 0.
>>>>>>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F
>>>>>>>>> spec, which is
>>>>>>>>> extended to (VIC 1-107).
>>>>>>>>>
>>>>>>>>> This patch adds a bool input variable, which indicates if
>>>>>>>>> the connected
>>>>>>>>> sink is a HDMI 2.0 sink or not. This will make sure that we
>>>>>>>>> don't pass a
>>>>>>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>>>>>>>>
>>>>>>>>> This patch touches all drm drivers, who are callers of this
>>>>>>>>> function
>>>>>>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure
>>>>>>>>> there is
>>>>>>>>> no change in current behavior, is_hdmi2 is kept as false.
>>>>>>>>>
>>>>>>>>> In case of I915 driver, this patch checks the
>>>>>>>>> connector->display_info
>>>>>>>>> to check if the connected display is HDMI 2.0.
>>>>>>>>>
>>>>>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>>>>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>>>>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>>>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>>>>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>>>>>>>
>>>>>>>>> PS: This patch touches a few lines in few files, which were
>>>>>>>>> already above 80 char, so checkpatch gives 80 char warning
>>>>>>>>> again.
>>>>>>>>> - gpu/drm/omapdrm/omap_encoder.c
>>>>>>>>> - gpu/drm/i915/intel_sdvo.c
>>>>>>>>>
>>>>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>>>>>> ---
>>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
>>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
>>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
>>>>>>>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
>>>>>>>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>>>>>>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>>>>>>>> drivers/gpu/drm/drm_edid.c | 12
>>>>>>>>> +++++++++++-
>>>>>>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
>>>>>>>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>>>>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>>>>>>>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
>>>>>>>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
>>>>>>>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
>>>>>>>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
>>>>>>>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>>>>>>>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
>>>>>>>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
>>>>>>>>> drivers/gpu/drm/tegra/sor.c | 2 +-
>>>>>>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
>>>>>>>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
>>>>>>>>> include/drm/drm_edid.h | 3 ++-
>>>>>>>>> 21 files changed, 38 insertions(+), 21 deletions(-)
>>>>>>>>>
>>>>>>>> [snip]
>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>>>> index af93f7a..5ff2886 100644
>>>>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct
>>>>>>>>> dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>>>>>>> u8 val;
>>>>>>>>> /* Initialise info frame from DRM mode */
>>>>>>>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>>>>>>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode,
>>>>>>>>> false);
>>>>>>>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>>>>>>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>>>>>>
>>>>>>>> dw-hdmi controller has full support for HDMI 2.0 features.
>>>>>>>> It all
>>>>>>>> depends on the platform it is integrated.
>>>>>>>>
>>>>>>>> I think adding a parameter to
>>>>>>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
>>>>>>>> because of this case: A bridge can have support for HDMI 2.0
>>>>>>>> features but the platform may limit this support. I guess it
>>>>>>>> can
>>>>>>>> happen in other drivers too.
>>>>>>> Your driver is in full control of what gets passed here. So I
>>>>>>> don't see
>>>>>>> why that would be a problem.
>>>>>>>
>>>>>>> Also this doesn't really have anything to do with the
>>>>>>> capabilities of
>>>>>>> the source. All we want to make sure is that we don't send a
>>>>>>> VIC the
>>>>>>> sink will not understand.
>>>>>>>
>>>>>> But the driver is not aware of the platform limitations, its
>>>>>> generic to the controller only. We could add a field in pdata
>>>>>> which tells if platform is HDMI 2.0+ but what about other
>>>>>> bridge
>>>>>> drivers or HDMI drivers? They will have to replicate the same
>>>>>> thing also.
>>>>>>
>>>>>> Best regards,
>>>>>> Jose Miguel Abreu
>>>>> I think the driver would be aware of the platform's
>>>>> capabilities, isn't it ?
>>>>> Else how would it even decide which mode to allow, and which to
>>>>> reject ?
>>>> The DRM core propagates the mode to the chain of configuration
>>>> before reaching the bridge driver also, there is a callback
>>>> supplied by pdata (mode_valid) which can check if the mode is
>>>> valid. (see
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_-7Eairlied_linux_tree_drivers_gpu_drm_bridge_synopsys_dw-2Dhdmi.c-3Fh-3Ddrm-2Dnext-23n1740&d=DwID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=zC1cWRy6RDWimQG2o866yYylV7c5h_U4aGWjVtnIHH0&s=DPZA3OgLWOO299hjy9dBDBK66kmUGfZArxKbeHNMKOc&e=
>>>> )
>>>>
>>>> Best regards,
>>>> Jose Miguel Abreu
>>>>
>>>>
>>>>> Regards
>>>>> Shashank
>>> Please correct me if my understanding is not right, but drivers
>>> call mode_valid() to prune/reject modes which they cant support.
>>> and they call drm_set_infoframe_from_videomode() function, when
>>> they are going ahead to the modeset with a mode.
>>> Why would a driver choose to do a modeset, which it could not
>>> support (shouldn't mode_valid have dropped it in atomic_check()
>>> or may be even edid_parsing time ?)
>> Yes, thats all correct :) I got a little out of topic here, sorry.
>>
>> The thing I don't agree with is the adding of the parameter for
>> the drm_hdmi_avi_infoframe_from_display_mode(), because I think
>> its not enough. I will state why:
>>
>> Picture this scenario:
>> 1) EDID is read from HDMI 2.0 sink
>> 2) The 2.0 modes are probed and as there is no check for
>> these modes (they are in the CEA table) they will be added to
>> mode list
>> 3) The modes are passed to userspace. This is a problem
>> because userspace does not yet understand the new aspect ratios,
>> but lets imagine it understands
>> 4) User selects an HDMI 2.0 mode
>> 5) The drm core checks if mode is valid with driver, the
>> driver can say yes or no. Lets think it says yes because the
>> platform supports HDMI 2.0 modes.
>> 6) Mode is committed but VIC will be set to zero according to
>> the patch
> That's only because you haven't updated your driver to pass 'true'
> as needed.
>
> The other option would be to pass the connector instead of a boolean,
> but according to Shashank that would required a lot of plumbing changes
> because many drivers don't have the connector immediately available
> where they call these functions. And plumbing the connector through
> probably requires intimate knowledge of each driver, and thus is not
> something we would be confortable doing. Hence I think the bool is the
> best short term solution, letting each driver author figure out how to
> get at the right connector and pass on the required information.
>
> I don't really understand what you're even suggesting as an alternative.
> We need to know what the sink supports, and if your driver is architected
> in some crazy way where you don't even have that information where it's
> needed, I don't think there's anything anyone can do.
>
I submitted a RFC yesterday where there was an alternative
[1][2][3][4][5][6]: Don't expose 2.0 modes to drivers and to
userspace unless asked to. The infoframe part is a different
topic though. We could add a helper in the DRM EDID which checks
if sink is 2.0 and then, in conjunction with the flag I
introduced in the RFC patch 4/5 we would know that driver
supports 2.0 and sink is 2.0, then we would eliminate the bool.
What do you think?
Best regards,
Jose Miguel Abreu
[1]
https://lists.freedesktop.org/archives/dri-devel/2017-March/136596.html
[2] https://patchwork.kernel.org/patch/9640215/
[3] https://patchwork.kernel.org/patch/9640219/
[4] https://patchwork.kernel.org/patch/9640207/
[5] https://patchwork.kernel.org/patch/9640217/
[6] https://patchwork.kernel.org/patch/9640205/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 17:53 ` Jose Abreu
@ 2017-03-23 18:14 ` Ville Syrjälä
2017-03-23 18:35 ` Jose Abreu
0 siblings, 1 reply; 22+ messages in thread
From: Ville Syrjälä @ 2017-03-23 18:14 UTC (permalink / raw)
To: Jose Abreu
Cc: intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher, Daniel Vetter
On Thu, Mar 23, 2017 at 05:53:34PM +0000, Jose Abreu wrote:
> Hi Ville,
>
>
> On 23-03-2017 17:42, Ville Syrjälä wrote:
> > On Thu, Mar 23, 2017 at 05:11:44PM +0000, Jose Abreu wrote:
> >> Hi Shashank,
> >>
> >>
> >> On 23-03-2017 16:43, Sharma, Shashank wrote:
> >>> Regards
> >>>
> >>> Shashank
> >>>
> >>>
> >>> On 3/23/2017 6:33 PM, Jose Abreu wrote:
> >>>> Hi Shashank,
> >>>>
> >>>>
> >>>> On 23-03-2017 16:08, Sharma, Shashank wrote:
> >>>>> Regards
> >>>>>
> >>>>> Shashank
> >>>>>
> >>>>>
> >>>>> On 3/23/2017 5:57 PM, Jose Abreu wrote:
> >>>>>> Hi Ville,
> >>>>>>
> >>>>>>
> >>>>>> On 23-03-2017 15:45, Ville Syrjälä wrote:
> >>>>>>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
> >>>>>>>> Hi Shashank,
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 23-03-2017 15:14, Shashank Sharma wrote:
> >>>>>>>>> HDMI 1.4b support the CEA video modes as per range of
> >>>>>>>>> CEA-861-D (VIC 1-64).
> >>>>>>>>> For any other mode, the VIC filed in AVI infoframes should
> >>>>>>>>> be 0.
> >>>>>>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F
> >>>>>>>>> spec, which is
> >>>>>>>>> extended to (VIC 1-107).
> >>>>>>>>>
> >>>>>>>>> This patch adds a bool input variable, which indicates if
> >>>>>>>>> the connected
> >>>>>>>>> sink is a HDMI 2.0 sink or not. This will make sure that we
> >>>>>>>>> don't pass a
> >>>>>>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
> >>>>>>>>>
> >>>>>>>>> This patch touches all drm drivers, who are callers of this
> >>>>>>>>> function
> >>>>>>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure
> >>>>>>>>> there is
> >>>>>>>>> no change in current behavior, is_hdmi2 is kept as false.
> >>>>>>>>>
> >>>>>>>>> In case of I915 driver, this patch checks the
> >>>>>>>>> connector->display_info
> >>>>>>>>> to check if the connected display is HDMI 2.0.
> >>>>>>>>>
> >>>>>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> >>>>>>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
> >>>>>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >>>>>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
> >>>>>>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
> >>>>>>>>>
> >>>>>>>>> PS: This patch touches a few lines in few files, which were
> >>>>>>>>> already above 80 char, so checkpatch gives 80 char warning
> >>>>>>>>> again.
> >>>>>>>>> - gpu/drm/omapdrm/omap_encoder.c
> >>>>>>>>> - gpu/drm/i915/intel_sdvo.c
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>>>>>>> ---
> >>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
> >>>>>>>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/drm_edid.c | 12
> >>>>>>>>> +++++++++++-
> >>>>>>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
> >>>>>>>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
> >>>>>>>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
> >>>>>>>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/tegra/sor.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
> >>>>>>>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> >>>>>>>>> include/drm/drm_edid.h | 3 ++-
> >>>>>>>>> 21 files changed, 38 insertions(+), 21 deletions(-)
> >>>>>>>>>
> >>>>>>>> [snip]
> >>>>>>>>
> >>>>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>>>> index af93f7a..5ff2886 100644
> >>>>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct
> >>>>>>>>> dw_hdmi *hdmi, struct drm_display_mode *mode)
> >>>>>>>>> u8 val;
> >>>>>>>>> /* Initialise info frame from DRM mode */
> >>>>>>>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> >>>>>>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode,
> >>>>>>>>> false);
> >>>>>>>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> >>>>>>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
> >>>>>>>>>
> >>>>>>>> dw-hdmi controller has full support for HDMI 2.0 features.
> >>>>>>>> It all
> >>>>>>>> depends on the platform it is integrated.
> >>>>>>>>
> >>>>>>>> I think adding a parameter to
> >>>>>>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
> >>>>>>>> because of this case: A bridge can have support for HDMI 2.0
> >>>>>>>> features but the platform may limit this support. I guess it
> >>>>>>>> can
> >>>>>>>> happen in other drivers too.
> >>>>>>> Your driver is in full control of what gets passed here. So I
> >>>>>>> don't see
> >>>>>>> why that would be a problem.
> >>>>>>>
> >>>>>>> Also this doesn't really have anything to do with the
> >>>>>>> capabilities of
> >>>>>>> the source. All we want to make sure is that we don't send a
> >>>>>>> VIC the
> >>>>>>> sink will not understand.
> >>>>>>>
> >>>>>> But the driver is not aware of the platform limitations, its
> >>>>>> generic to the controller only. We could add a field in pdata
> >>>>>> which tells if platform is HDMI 2.0+ but what about other
> >>>>>> bridge
> >>>>>> drivers or HDMI drivers? They will have to replicate the same
> >>>>>> thing also.
> >>>>>>
> >>>>>> Best regards,
> >>>>>> Jose Miguel Abreu
> >>>>> I think the driver would be aware of the platform's
> >>>>> capabilities, isn't it ?
> >>>>> Else how would it even decide which mode to allow, and which to
> >>>>> reject ?
> >>>> The DRM core propagates the mode to the chain of configuration
> >>>> before reaching the bridge driver also, there is a callback
> >>>> supplied by pdata (mode_valid) which can check if the mode is
> >>>> valid. (see
> >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_-7Eairlied_linux_tree_drivers_gpu_drm_bridge_synopsys_dw-2Dhdmi.c-3Fh-3Ddrm-2Dnext-23n1740&d=DwID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=zC1cWRy6RDWimQG2o866yYylV7c5h_U4aGWjVtnIHH0&s=DPZA3OgLWOO299hjy9dBDBK66kmUGfZArxKbeHNMKOc&e=
> >>>> )
> >>>>
> >>>> Best regards,
> >>>> Jose Miguel Abreu
> >>>>
> >>>>
> >>>>> Regards
> >>>>> Shashank
> >>> Please correct me if my understanding is not right, but drivers
> >>> call mode_valid() to prune/reject modes which they cant support.
> >>> and they call drm_set_infoframe_from_videomode() function, when
> >>> they are going ahead to the modeset with a mode.
> >>> Why would a driver choose to do a modeset, which it could not
> >>> support (shouldn't mode_valid have dropped it in atomic_check()
> >>> or may be even edid_parsing time ?)
> >> Yes, thats all correct :) I got a little out of topic here, sorry.
> >>
> >> The thing I don't agree with is the adding of the parameter for
> >> the drm_hdmi_avi_infoframe_from_display_mode(), because I think
> >> its not enough. I will state why:
> >>
> >> Picture this scenario:
> >> 1) EDID is read from HDMI 2.0 sink
> >> 2) The 2.0 modes are probed and as there is no check for
> >> these modes (they are in the CEA table) they will be added to
> >> mode list
> >> 3) The modes are passed to userspace. This is a problem
> >> because userspace does not yet understand the new aspect ratios,
> >> but lets imagine it understands
> >> 4) User selects an HDMI 2.0 mode
> >> 5) The drm core checks if mode is valid with driver, the
> >> driver can say yes or no. Lets think it says yes because the
> >> platform supports HDMI 2.0 modes.
> >> 6) Mode is committed but VIC will be set to zero according to
> >> the patch
> > That's only because you haven't updated your driver to pass 'true'
> > as needed.
> >
> > The other option would be to pass the connector instead of a boolean,
> > but according to Shashank that would required a lot of plumbing changes
> > because many drivers don't have the connector immediately available
> > where they call these functions. And plumbing the connector through
> > probably requires intimate knowledge of each driver, and thus is not
> > something we would be confortable doing. Hence I think the bool is the
> > best short term solution, letting each driver author figure out how to
> > get at the right connector and pass on the required information.
> >
> > I don't really understand what you're even suggesting as an alternative.
> > We need to know what the sink supports, and if your driver is architected
> > in some crazy way where you don't even have that information where it's
> > needed, I don't think there's anything anyone can do.
> >
>
> I submitted a RFC yesterday where there was an alternative
> [1][2][3][4][5][6]: Don't expose 2.0 modes to drivers and to
> userspace unless asked to.
I don't think that's a good plan. We already hit a massive snag with
the aspect ratio uapi changes. So we should aim for something that
requires no userspace changes. For this stuff I see no reason to
bother userspace with these details. A mode is just a mode, whether
it was specified in HDMI 2.0 or DMT or CVT is quite irrelevant.
> The infoframe part is a different
> topic though. We could add a helper in the DRM EDID which checks
> if sink is 2.0 and then,
But that needs the connector, which some drivers don't have ready
at hand. And we already have that information pre-parsed in the
display_info so we pretty much already have what you suggest.
The only thing is that we require the driver to get that information
since the infoframe code doesn't know how to do that for every
driver.
> in conjunction with the flag I
> introduced in the RFC patch 4/5 we would know that driver
> supports 2.0 and sink is 2.0, then we would eliminate the bool.
> What do you think?
>
> Best regards,
> Jose Miguel Abreu
>
> [1]
> https://lists.freedesktop.org/archives/dri-devel/2017-March/136596.html
> [2] https://patchwork.kernel.org/patch/9640215/
> [3] https://patchwork.kernel.org/patch/9640219/
> [4] https://patchwork.kernel.org/patch/9640207/
> [5] https://patchwork.kernel.org/patch/9640217/
> [6] https://patchwork.kernel.org/patch/9640205/
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames
2017-03-23 18:14 ` Ville Syrjälä
@ 2017-03-23 18:35 ` Jose Abreu
0 siblings, 0 replies; 22+ messages in thread
From: Jose Abreu @ 2017-03-23 18:35 UTC (permalink / raw)
To: Ville Syrjälä, Jose Abreu
Cc: intel-gfx, dri-devel, Andrzej Hajda, Alex Deucher, Daniel Vetter
Hi Ville,
On 23-03-2017 18:14, Ville Syrjälä wrote:
> On Thu, Mar 23, 2017 at 05:53:34PM +0000, Jose Abreu wrote:
>> Hi Ville,
>>
>>
>> On 23-03-2017 17:42, Ville Syrjälä wrote:
>>> On Thu, Mar 23, 2017 at 05:11:44PM +0000, Jose Abreu wrote:
>>>> Hi Shashank,
>>>>
>>>>
>>>> On 23-03-2017 16:43, Sharma, Shashank wrote:
>>>>> Regards
>>>>>
>>>>> Shashank
>>>>>
>>>>>
>>>>> On 3/23/2017 6:33 PM, Jose Abreu wrote:
>>>>>> Hi Shashank,
>>>>>>
>>>>>>
>>>>>> On 23-03-2017 16:08, Sharma, Shashank wrote:
>>>>>>> Regards
>>>>>>>
>>>>>>> Shashank
>>>>>>>
>>>>>>>
>>>>>>> On 3/23/2017 5:57 PM, Jose Abreu wrote:
>>>>>>>> Hi Ville,
>>>>>>>>
>>>>>>>>
>>>>>>>> On 23-03-2017 15:45, Ville Syrjälä wrote:
>>>>>>>>> On Thu, Mar 23, 2017 at 03:28:29PM +0000, Jose Abreu wrote:
>>>>>>>>>> Hi Shashank,
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 23-03-2017 15:14, Shashank Sharma wrote:
>>>>>>>>>>> HDMI 1.4b support the CEA video modes as per range of
>>>>>>>>>>> CEA-861-D (VIC 1-64).
>>>>>>>>>>> For any other mode, the VIC filed in AVI infoframes should
>>>>>>>>>>> be 0.
>>>>>>>>>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F
>>>>>>>>>>> spec, which is
>>>>>>>>>>> extended to (VIC 1-107).
>>>>>>>>>>>
>>>>>>>>>>> This patch adds a bool input variable, which indicates if
>>>>>>>>>>> the connected
>>>>>>>>>>> sink is a HDMI 2.0 sink or not. This will make sure that we
>>>>>>>>>>> don't pass a
>>>>>>>>>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>>>>>>>>>>
>>>>>>>>>>> This patch touches all drm drivers, who are callers of this
>>>>>>>>>>> function
>>>>>>>>>>> drm_hdmi_avi_infoframe_from_display_mode but to make sure
>>>>>>>>>>> there is
>>>>>>>>>>> no change in current behavior, is_hdmi2 is kept as false.
>>>>>>>>>>>
>>>>>>>>>>> In case of I915 driver, this patch checks the
>>>>>>>>>>> connector->display_info
>>>>>>>>>>> to check if the connected display is HDMI 2.0.
>>>>>>>>>>>
>>>>>>>>>>> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>>>>>>>>>> Cc: Jose Abreu <jose.abreu@synopsys.com>
>>>>>>>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>>>>>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>>>>>>>>> Cc: Daniel Vetter <daniel.vetter@intel.com>
>>>>>>>>>>>
>>>>>>>>>>> PS: This patch touches a few lines in few files, which were
>>>>>>>>>>> already above 80 char, so checkpatch gives 80 char warning
>>>>>>>>>>> again.
>>>>>>>>>>> - gpu/drm/omapdrm/omap_encoder.c
>>>>>>>>>>> - gpu/drm/i915/intel_sdvo.c
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>>>>>>>> ---
>>>>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
>>>>>>>>>>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/drm_edid.c | 12
>>>>>>>>>>> +++++++++++-
>>>>>>>>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>>>>>>>>>>> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
>>>>>>>>>>> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/omapdrm/omap_encoder.c | 3 ++-
>>>>>>>>>>> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/tegra/hdmi.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/tegra/sor.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/vc4/vc4_hdmi.c | 2 +-
>>>>>>>>>>> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
>>>>>>>>>>> include/drm/drm_edid.h | 3 ++-
>>>>>>>>>>> 21 files changed, 38 insertions(+), 21 deletions(-)
>>>>>>>>>>>
>>>>>>>>>> [snip]
>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>>>>>> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>>>>>> index af93f7a..5ff2886 100644
>>>>>>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>>>>>> @@ -1146,7 +1146,7 @@ static void hdmi_config_AVI(struct
>>>>>>>>>>> dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>>>>>>>>> u8 val;
>>>>>>>>>>> /* Initialise info frame from DRM mode */
>>>>>>>>>>> - drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
>>>>>>>>>>> + drm_hdmi_avi_infoframe_from_display_mode(&frame, mode,
>>>>>>>>>>> false);
>>>>>>>>>>> if (hdmi->hdmi_data.enc_out_format == YCBCR444)
>>>>>>>>>>> frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>>>>>>>>
>>>>>>>>>> dw-hdmi controller has full support for HDMI 2.0 features.
>>>>>>>>>> It all
>>>>>>>>>> depends on the platform it is integrated.
>>>>>>>>>>
>>>>>>>>>> I think adding a parameter to
>>>>>>>>>> drm_hdmi_avi_infoframe_from_display_mode is not the best idea
>>>>>>>>>> because of this case: A bridge can have support for HDMI 2.0
>>>>>>>>>> features but the platform may limit this support. I guess it
>>>>>>>>>> can
>>>>>>>>>> happen in other drivers too.
>>>>>>>>> Your driver is in full control of what gets passed here. So I
>>>>>>>>> don't see
>>>>>>>>> why that would be a problem.
>>>>>>>>>
>>>>>>>>> Also this doesn't really have anything to do with the
>>>>>>>>> capabilities of
>>>>>>>>> the source. All we want to make sure is that we don't send a
>>>>>>>>> VIC the
>>>>>>>>> sink will not understand.
>>>>>>>>>
>>>>>>>> But the driver is not aware of the platform limitations, its
>>>>>>>> generic to the controller only. We could add a field in pdata
>>>>>>>> which tells if platform is HDMI 2.0+ but what about other
>>>>>>>> bridge
>>>>>>>> drivers or HDMI drivers? They will have to replicate the same
>>>>>>>> thing also.
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Jose Miguel Abreu
>>>>>>> I think the driver would be aware of the platform's
>>>>>>> capabilities, isn't it ?
>>>>>>> Else how would it even decide which mode to allow, and which to
>>>>>>> reject ?
>>>>>> The DRM core propagates the mode to the chain of configuration
>>>>>> before reaching the bridge driver also, there is a callback
>>>>>> supplied by pdata (mode_valid) which can check if the mode is
>>>>>> valid. (see
>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_-7Eairlied_linux_tree_drivers_gpu_drm_bridge_synopsys_dw-2Dhdmi.c-3Fh-3Ddrm-2Dnext-23n1740&d=DwID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=zC1cWRy6RDWimQG2o866yYylV7c5h_U4aGWjVtnIHH0&s=DPZA3OgLWOO299hjy9dBDBK66kmUGfZArxKbeHNMKOc&e=
>>>>>> )
>>>>>>
>>>>>> Best regards,
>>>>>> Jose Miguel Abreu
>>>>>>
>>>>>>
>>>>>>> Regards
>>>>>>> Shashank
>>>>> Please correct me if my understanding is not right, but drivers
>>>>> call mode_valid() to prune/reject modes which they cant support.
>>>>> and they call drm_set_infoframe_from_videomode() function, when
>>>>> they are going ahead to the modeset with a mode.
>>>>> Why would a driver choose to do a modeset, which it could not
>>>>> support (shouldn't mode_valid have dropped it in atomic_check()
>>>>> or may be even edid_parsing time ?)
>>>> Yes, thats all correct :) I got a little out of topic here, sorry.
>>>>
>>>> The thing I don't agree with is the adding of the parameter for
>>>> the drm_hdmi_avi_infoframe_from_display_mode(), because I think
>>>> its not enough. I will state why:
>>>>
>>>> Picture this scenario:
>>>> 1) EDID is read from HDMI 2.0 sink
>>>> 2) The 2.0 modes are probed and as there is no check for
>>>> these modes (they are in the CEA table) they will be added to
>>>> mode list
>>>> 3) The modes are passed to userspace. This is a problem
>>>> because userspace does not yet understand the new aspect ratios,
>>>> but lets imagine it understands
>>>> 4) User selects an HDMI 2.0 mode
>>>> 5) The drm core checks if mode is valid with driver, the
>>>> driver can say yes or no. Lets think it says yes because the
>>>> platform supports HDMI 2.0 modes.
>>>> 6) Mode is committed but VIC will be set to zero according to
>>>> the patch
>>> That's only because you haven't updated your driver to pass 'true'
>>> as needed.
>>>
>>> The other option would be to pass the connector instead of a boolean,
>>> but according to Shashank that would required a lot of plumbing changes
>>> because many drivers don't have the connector immediately available
>>> where they call these functions. And plumbing the connector through
>>> probably requires intimate knowledge of each driver, and thus is not
>>> something we would be confortable doing. Hence I think the bool is the
>>> best short term solution, letting each driver author figure out how to
>>> get at the right connector and pass on the required information.
>>>
>>> I don't really understand what you're even suggesting as an alternative.
>>> We need to know what the sink supports, and if your driver is architected
>>> in some crazy way where you don't even have that information where it's
>>> needed, I don't think there's anything anyone can do.
>>>
>> I submitted a RFC yesterday where there was an alternative
>> [1][2][3][4][5][6]: Don't expose 2.0 modes to drivers and to
>> userspace unless asked to.
> I don't think that's a good plan. We already hit a massive snag with
> the aspect ratio uapi changes. So we should aim for something that
> requires no userspace changes. For this stuff I see no reason to
> bother userspace with these details. A mode is just a mode, whether
> it was specified in HDMI 2.0 or DMT or CVT is quite irrelevant.
I agree that we should aim for no userspace changes but sometimes
they are necessary. Anyway, I was forgetting that the new aspect
ratios will not be exported, so I guess we are safe for now.
>
>> The infoframe part is a different
>> topic though. We could add a helper in the DRM EDID which checks
>> if sink is 2.0 and then,
> But that needs the connector, which some drivers don't have ready
> at hand. And we already have that information pre-parsed in the
> display_info so we pretty much already have what you suggest.
> The only thing is that we require the driver to get that information
> since the infoframe code doesn't know how to do that for every
> driver.
Right, so, no connector :/ I can check if dw-hdmi is in this
condition and send a patch to adjust the flag.
Best regards,
Jose Miguel Abreu
>
>> in conjunction with the flag I
>> introduced in the RFC patch 4/5 we would know that driver
>> supports 2.0 and sink is 2.0, then we would eliminate the bool.
>> What do you think?
>>
>> Best regards,
>> Jose Miguel Abreu
>>
>> [1]
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_archives_dri-2Ddevel_2017-2DMarch_136596.html&d=DwIDAw&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=IHo0Vkqnru9M6CloOKR-DrbSSmwnEHmThqrw9onMZ2o&s=7cX3HPDl6xffu7Zfoz30XBwhOyvIZ1RzTTb1z35ziV0&e=
>> [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9640215_&d=DwIDAw&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=IHo0Vkqnru9M6CloOKR-DrbSSmwnEHmThqrw9onMZ2o&s=ZHn9oCaMsLmPoaHbthOZXA8x8ixy2jik_3ZZRrjNtOs&e=
>> [3] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9640219_&d=DwIDAw&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=IHo0Vkqnru9M6CloOKR-DrbSSmwnEHmThqrw9onMZ2o&s=Xk-h91zq4XWDElWy9unrTER7BgryuUyKBb6dOusgAVw&e=
>> [4] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9640207_&d=DwIDAw&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=IHo0Vkqnru9M6CloOKR-DrbSSmwnEHmThqrw9onMZ2o&s=ByJ1jqHxbvkcZp64T1FFO9C_MljhDpJzVQgPIywiQ74&e=
>> [5] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9640217_&d=DwIDAw&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=IHo0Vkqnru9M6CloOKR-DrbSSmwnEHmThqrw9onMZ2o&s=giBMu5vbko6VWhq-Ob90DDBhEcpRqFLeSgZmeNnOEX8&e=
>> [6] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9640205_&d=DwIDAw&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=IHo0Vkqnru9M6CloOKR-DrbSSmwnEHmThqrw9onMZ2o&s=cAN-hG4zuHeWTJSYg1ID2hCprnYuKeZDDtxIKsy81j8&e=
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-03-23 18:35 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 15:14 [PATCH v4 1/2] drm/edid: Complete CEA modedb(VIC 1-107) Shashank Sharma
2017-03-23 15:14 ` [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames Shashank Sharma
2017-03-23 15:17 ` Ilia Mirkin
2017-03-23 15:22 ` Sharma, Shashank
2017-03-23 15:28 ` Ilia Mirkin
2017-03-23 16:33 ` Sharma, Shashank
2017-03-23 15:28 ` Jose Abreu
2017-03-23 15:45 ` Ville Syrjälä
2017-03-23 15:57 ` Jose Abreu
2017-03-23 16:08 ` Sharma, Shashank
2017-03-23 16:33 ` Jose Abreu
2017-03-23 16:43 ` Sharma, Shashank
2017-03-23 17:11 ` Jose Abreu
2017-03-23 17:42 ` Ville Syrjälä
2017-03-23 17:53 ` Jose Abreu
2017-03-23 18:14 ` Ville Syrjälä
2017-03-23 18:35 ` Jose Abreu
2017-03-23 15:36 ` Ville Syrjälä
2017-03-23 15:52 ` Jose Abreu
2017-03-23 16:31 ` Sharma, Shashank
2017-03-23 17:01 ` Ville Syrjälä
2017-03-23 16:01 ` ✓ Fi.CI.BAT: success for series starting with [v4,1/2] drm/edid: Complete CEA modedb(VIC 1-107) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.