| Message ID | 1490282059-16331-2-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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?
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
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.
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
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
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
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
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
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
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
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.
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
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.
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/
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/
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=
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);
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(-)