Re: [PATCH 2/2] drm/hdmi: Allow HDMI infoframe without VIC or S3D

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jul 03, 2017 at 10:19:38PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Appedix F of HDMI 2.0 says that some HDMI sink may fail to switch from
> 3D to 2D mode in a timely fashion if the source simply stops sending the
> HDMI infoframe. The suggested workaround is to keep sending the
> infoframe even when strictly not necessary (ie. no VIC and no S3D).
> HDMI 1.4 does allow for this behaviour, stating that sending the
> infoframe is optional in this case.
> 
> The infoframe was first specified in HDMI 1.4, so in theory sinks
> predating that may not appreciate us sending an uknown infoframe
> their way. To avoid regressions let's try to determine if the sink
> supports the infoframe or not. Unfortunately there's no direct way
> to do that, so instead we'll just check if we managed to parse any
> HDMI 1.4 4k or stereo modes from the EDID, and if so we assume the
> sink will accept the infoframe. Also if the EDID contains the HDMI
> 2.0 HDMI Forum VSDB we can assume the sink is prepared to receive
> the infoframe.
> 
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Mark Yao <mark.yao@rock-chips.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 ++-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  4 +++-
>  drivers/gpu/drm/drm_edid.c                | 32 +++++++++++++++++++++++++------
>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  2 +-
>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 ++++++++------
>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
>  drivers/gpu/drm/nouveau/nv50_display.c    |  3 ++-
>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  1 +
>  drivers/gpu/drm/sti/sti_hdmi.c            |  4 +++-
>  drivers/gpu/drm/zte/zx_hdmi.c             |  1 +
>  include/drm/drm_connector.h               |  5 +++++
>  include/drm/drm_edid.h                    |  1 +
>  12 files changed, 55 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 2d51a2269fc6..758b5a4546f1 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -2136,8 +2136,9 @@ static bool sii8620_mode_fixup(struct drm_bridge *bridge,
>  			union hdmi_infoframe frm;
>  			u8 mhl_vic[] = { 0, 95, 94, 93, 98 };
>  
> +			/* FIXME: We need the connector here */
>  			drm_hdmi_vendor_infoframe_from_display_mode(
> -				&frm.vendor.hdmi, adjusted_mode);
> +				&frm.vendor.hdmi, NULL, adjusted_mode);
>  			vic = frm.vendor.hdmi.vic;
>  			if (vic >= ARRAY_SIZE(mhl_vic))
>  				vic = 0;
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index ead11242c4b9..c43389774691 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1426,7 +1426,9 @@ static void hdmi_config_vendor_specific_infoframe(struct dw_hdmi *hdmi,
>  	u8 buffer[10];
>  	ssize_t err;
>  
> -	err = drm_hdmi_vendor_infoframe_from_display_mode(&frame, mode);
> +	err = drm_hdmi_vendor_infoframe_from_display_mode(&frame,
> +							  &hdmi->connector,
> +							  mode);
>  	if (err < 0)
>  		/*
>  		 * Going into that statement does not means vendor infoframe
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 2e55599816aa..c061dd5d25c0 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3081,6 +3081,7 @@ static int
>  do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
>  		   const u8 *video_db, u8 video_len)
>  {
> +	struct drm_display_info *info = &connector->display_info;
>  	int modes = 0, offset = 0, i, multi_present = 0, multi_len;
>  	u8 vic_len, hdmi_3d_len = 0;
>  	u16 mask;
> @@ -3208,6 +3209,8 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len,
>  	}
>  
>  out:
> +	if (modes > 0)
> +		info->has_hdmi_infoframe = true;

Oh, and I forgot to mention that this depends on Shashank's patch to
reorder things such that drm_add_display_info() gets called prior to
parsing the modes. With the current order we'd end up clearing this
almost immediately afterwards.

>  	return modes;
>  }
>  
> @@ -3829,6 +3832,8 @@ static void drm_parse_hdmi_forum_vsdb(struct drm_connector *connector,
>  	struct drm_display_info *display = &connector->display_info;
>  	struct drm_hdmi_info *hdmi = &display->hdmi;
>  
> +	display->has_hdmi_infoframe = true;
> +
>  	if (hf_vsdb[6] & 0x80) {
>  		hdmi->scdc.supported = true;
>  		if (hf_vsdb[6] & 0x40)
> @@ -3998,6 +4003,7 @@ static void drm_add_display_info(struct drm_connector *connector,
>  	info->cea_rev = 0;
>  	info->max_tmds_clock = 0;
>  	info->dvi_dual = false;
> +	info->has_hdmi_infoframe = false;
>  
>  	if (edid->revision < 3)
>  		return;

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx