On Tue, May 30, 2017 at 10:00:12PM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 5/30/2017 9:43 PM, Ville Syrjälä wrote: > > On Tue, May 30, 2017 at 05:43:40PM +0530, 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 > >> > >> V2: Rebase, Added r-b from Andrzej > >> > >> Reviewed-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx> > >> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx> > >> --- > >> 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 3c62c45..4923ddc 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c > >> @@ -1864,7 +1864,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 c8ed0fa..4101684 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c > >> @@ -1848,7 +1848,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 3e90c19..a7f6b32 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > >> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c > >> @@ -1747,7 +1747,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 9b87067..3dc40f6 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 8737de8..e6d3181 100644 > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > >> @@ -1317,7 +1317,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 2e55599..8b23435 100644 > >> --- a/drivers/gpu/drm/drm_edid.c > >> +++ b/drivers/gpu/drm/drm_edid.c > >> @@ -4334,12 +4334,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; > >> > >> @@ -4355,6 +4357,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; > > Seems I've totally forgotten what we concluded about the AVI VIC > > vs. HDMI VIC, but I have a feeling this isn't sufficient. > As far as I remember, we decided to send VICs > 64 to HDMI 2.0 monitors > only. > If you can provide some more thoughts about how can we make it better ? The question is whether we send the VIC in the AVI infoframe of the HDMI infoframe. IIRC sending both isn't legal, and I think we might have concluded that we might have to pass the AVI infoframe to the HDMI infoframe construction function or vice versa to figure out which gets sent. > > - Shashank > >> + > >> 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 1ff6ab6..f2cc75d 100644 > >> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > >> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > >> @@ -784,7 +784,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 58d6903..702cbab 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 6cc1812..62d727a 100644 > >> --- a/drivers/gpu/drm/i915/intel_sdvo.c > >> +++ b/drivers/gpu/drm/i915/intel_sdvo.c > >> @@ -1006,7 +1006,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 a59c95a..dbc6a19 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 ed63d4e..406d6d8 100644 > >> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > >> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > >> @@ -395,7 +395,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); > >> -- > >> 2.7.4 -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel