Re: [PATCH 01/11] drm: Add HDMI 2.0 VIC support for AVI info-frames

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Regards

Shashank


On 4/10/2017 3:17 PM, Andrzej Hajda wrote:

On 07.04.2017 18:39, 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@xxxxxxxxxxxxxxx>
Cc: Jose Abreu <jose.abreu@xxxxxxxxxxxx>
Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>

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@xxxxxxxxx>

Finally I have chances to look at the specs and I am not sure if this
solution fully reflects the specs and is scalable. According to specs
VIC set to 0 in AVIF means "Video Format not documented in CTA-861", ie
it is described by vendor specific data block and vendor specific
infoframe, maybe something else(???).

It says "Video Format not documented in CEA-861 (means this version)"

I suppose ideally during EDID read
there should be recorded info for every mode if it was defined by vendor
extension, or not. This info could be later used by drivers to configure
AVIF and VSIF accordingly.
I don't think this has to do anything with it, we are simply checking if the monitor is HDMI 2.0 capable, coz if it is, it would be complaint to CEA-861-F and it would understand the VIC>64. If not && VIC>64, we will send VIC as 0, coz this VIC is defined in CEA-861-F spec. So I don't see the confusion.
You are welcome to suggest, how can we make it more scalable, or improve on it. We can always send revised patch set :-) 

- Shashank


Anyway as a short-term initial solution it could work.
So:
Reviewed-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx>

  --
Regards
Andrzej


---
  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 daf003d..5dc3e95 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 3a72967..b70f077 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 6943f26..bcf9c75 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 3bc856c..54ada40 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1286,7 +1286,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_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format)) 
  		frame.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fad3d44..f982a42 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4326,12 +4326,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;
@@ -4347,6 +4349,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 6efc3cb..76d9e0d 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -459,11 +459,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 41a1c03..a1770dc 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 aaacac1..770e31f 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 7d9b75e..7149968 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 0df7366..7e834e3 100644
--- a/drivers/gpu/drm/zte/zx_hdmi.c
+++ b/drivers/gpu/drm/zte/zx_hdmi.c
@@ -124,7 +124,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 7b9f48b..c07eb81 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -343,7 +343,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);




_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux