edid: Allow HDMI infoframe without VIC or S3D

Sharma, Shashank Thu, 16 Nov 2017 19:11:16 -0800

Regards

Shashank



On 11/16/2017 9:51 PM, Ville Syrjälä wrote:

On Thu, Nov 16, 2017 at 08:10:55PM +0530, Sharma, Shashank wrote:

Regards

Shashank


On 11/13/2017 10:34 PM, Ville Syrjala wrote:

From: Ville Syrjälä <ville.syrj...@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.

I am trying to get some sense from HDMI 2.0 spec section 10.2.1, which
talks about
switch from 3D to 2D.  To me it looks like:
If (sending_to_hdmi2_sinks) {
      - for 3d modes send HF-VSIF
      - for 2d modes && defined in H14b HFVSIF, send H14B-VSIF
        When you switch from 3d->2d {
           - send_HF-VSIF with 3D_valid bit = 0/1
       }
} else { /* HDMI 1.4b sinks from Appendix F */
      -  send H14b-VSIF with HDMI_video_format[2:0 = 0 OR 1]
}

Should we add a is_hdmi2 check and separate these cases ?

We don't support the HDMI forum infoframe. Maybe someone forgot to
implement that when adding the rest of HDMI 2.0 support? ;)

How to make an 'embarrassed smile ' smiley :) ?
Will start looking into it. Meanwhile
Reviewed-by: Shashank Sharma <shashank.sha...@intel.com>

v2: Fix the getting has_hdmi_infoframe from display_info
      Always fail constructing the infoframe if the display
      possibly can't handle it

Cc: Shashank Sharma <shashank.sha...@intel.com>
Cc: Andrzej Hajda <a.ha...@samsung.com>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Signed-off-by: Ville Syrjälä <ville.syrj...@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                | 34 
+++++++++++++++++++++++++------
   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, 57 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
b/drivers/gpu/drm/bridge/sil-sii8620.c
index b7eb704d0a8a..4417276ba02e 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -2220,8 +2220,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 a64ce7112288..b172139502d6 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1437,7 +1437,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 749d07a01772..9ada0ccf50df 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3393,6 +3393,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;
@@ -3520,6 +3521,8 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const 
u8 *db, u8 len,
        }

out:

+       if (modes > 0)
+               info->has_hdmi_infoframe = true;
        return modes;
   }

@@ -4243,6 +4246,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)
@@ -4416,6 +4421,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;
@@ -4903,6 +4909,7 @@ s3d_structure_from_display_mode(const struct 
drm_display_mode *mode)
    * drm_hdmi_vendor_infoframe_from_display_mode() - fill an HDMI infoframe 
with
    * data from a DRM display mode
    * @frame: HDMI vendor infoframe
+ * @connector: the connector

I remember our old discussion where we realized that we will need this
connector for all types of infoframes,
eventually, do you think we should start thinking about creating a
wrapper like drm_any_infoframe_from_display_mode(),
and pass connector and desired type to it ?Or I am getting too ambitious
targeting this series :) ?

    * @mode: DRM display mode
    *
    * Note that there's is a need to send HDMI vendor infoframes only when 
using a
@@ -4913,8 +4920,15 @@ s3d_structure_from_display_mode(const struct 
drm_display_mode *mode)
    */
   int
   drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe 
*frame,
+                                           struct drm_connector *connector,
                                            const struct drm_display_mode *mode)
   {
+       /*
+        * FIXME: sil-sii8620 doesn't have a connector around when
+        * we need one, so we have to be prepared for a NULL connector.
+        */
+       bool has_hdmi_infoframe = connector ?
+               connector->display_info.has_hdmi_infoframe : false;
        int err;
        u32 s3d_flags;
        u8 vic;
@@ -4922,11 +4936,21 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct 
hdmi_vendor_infoframe *frame,
        if (!frame || !mode)
                return -EINVAL;

+ if (!has_hdmi_infoframe)

+               return -EINVAL;
+
        vic = drm_match_hdmi_mode(mode);
        s3d_flags = mode->flags & DRM_MODE_FLAG_3D_MASK;

- if (!vic && !s3d_flags)

-               return -EINVAL;
+       /*
+        * Even if it's not absolutely necessary to send the infoframe
+        * (ie.vic==0 and s3d_struct==0) we will still send it if we
+        * know that the sink can handle it. This is based on a
+        * suggestion in HDMI 2.0 Appendix F. Apparently some sinks
+        * have trouble realizing that they shuld switch from 3D to 2D
+        * mode if the source simply stops sending the infoframe when
+        * it wants to switch from 3D to 2D.
+        */

if (vic && s3d_flags)

                return -EINVAL;
@@ -4935,10 +4959,8 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct 
hdmi_vendor_infoframe *frame,
        if (err < 0)
                return err;

- if (vic)

-               frame->vic = vic;
-       else
-               frame->s3d_struct = s3d_structure_from_display_mode(mode);
+       frame->vic = vic;
+       frame->s3d_struct = s3d_structure_from_display_mode(mode);

return 0;

   }
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 0109ff40b1db..812b2773ed69 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -795,7 +795,7 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
        }

ret = drm_hdmi_vendor_infoframe_from_display_mode(&frm.vendor.hdmi,

-                       &hdata->current_mode);
+                       &hdata->connector, &hdata->current_mode);
        if (!ret)
                ret = hdmi_vendor_infoframe_pack(&frm.vendor.hdmi, buf,
                                sizeof(buf));
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 2d95db64cdf2..2ccba4ccf7ad 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -512,12 +512,14 @@ static void intel_hdmi_set_spd_infoframe(struct 
drm_encoder *encoder,

static void

   intel_hdmi_set_hdmi_infoframe(struct drm_encoder *encoder,
-                             const struct intel_crtc_state *crtc_state)
+                             const struct intel_crtc_state *crtc_state,
+                             const struct drm_connector_state *conn_state)
   {
        union hdmi_infoframe frame;
        int ret;

ret = drm_hdmi_vendor_infoframe_from_display_mode(&frame.vendor.hdmi,

+                                                         conn_state->connector,
                                                          
&crtc_state->base.adjusted_mode);
        if (ret < 0)
                return;
@@ -584,7 +586,7 @@ static void g4x_set_infoframes(struct drm_encoder *encoder,