On Wed, Jul 05, 2017 at 11:46:14AM +0300, Laurent Pinchart wrote:
> Hi Ville,
>
> On Tuesday 04 Jul 2017 15:44:02 Ville Syrjälä wrote:
> > On Tue, Jul 04, 2017 at 01:56:07PM +0200, Andrzej Hajda wrote:
> > > On 03.07.2017 21:19, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > >> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >>
> > >> 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.
> > >
> > > My impression from the specs is that it should be done only after
> > > switching from 3d to 2d mode.
> > > In such case we just need to remember previous mode, if it was 3d, empty
> > > VSIF infoframe should be still generated for 2seconds.
> > > No need to do guesses from EDID.
> > > Am I right, or just missing something?
> >
> > This code has no idea about any 3D->2D transitions, trying to make it
> > do that would just result in a lot of complexity. Much easier to just
> > always send the infoframe.
> >
> > >> 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@xxxxxxxxxxxxxx>
> > >> Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
> > >> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx>
> > >> Cc: Inki Dae <inki.dae@xxxxxxxxxxx>
> > >> Cc: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> > >> Cc: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx>
> > >> Cc: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > >> Cc: CK Hu <ck.hu@xxxxxxxxxxxx>
> > >> Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > >> Cc: Ben Skeggs <bskeggs@xxxxxxxxxx>
> > >> Cc: Mark Yao <mark.yao@xxxxxxxxxxxxxx>
> > >> Cc: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx>
> > >> Cc: Vincent Abriou <vincent.abriou@xxxxxx>
> > >> Cc: Shawn Guo <shawnguo@xxxxxxxxxx>
> > >> Cc: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> > >> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >> ---
> > >>
> > >> 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(-)
>
> [snip]
>
> > > > 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
>
> [snip]
>
> > >> @@ -4452,6 +4458,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
> > >> * @mode: DRM display mode
> > >> *
> > >> * Note that there's is a need to send HDMI vendor infoframes only when
> > >> using a
> > >> @@ -4462,8 +4469,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 : NULL;
> > >
> > > I wonder if requiring connector is a good idea, I can imagine that this
> > > function can be necessary in pure drm_encoder or non-terminal drm_bridge.
> >
> > No decent way around it, unless we want to risk sending the infoframe to
> > pre HDMI 1.4 sinks. Sounds like you have some kind of layering problem
> > if you can't get at the connector when you call this.
>
> I think you have a layering violation if you assume that there's a connector
> after the HDMI encoder :-)
It doesn't have to be immediately after it.
> What if the encoder's output is connected to, let's
> say, an HDMI to DSI bridge ? There's no connector in that case.
The connector must be somewhere. Otherwise what's the point.
>
> > > > int err;
> > > > u32 s3d_flags;
> > > > u8 vic;
>
> [snip]
>
> --
> Regards,
>
> Laurent Pinchart
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
